tech@mandoc.bsd.lv
 help / color / mirror / Atom feed
* implement .so
@ 2010-10-24 16:40 Ingo Schwarze
  2010-10-24 16:49 ` Joerg Sonnenberger
  0 siblings, 1 reply; 19+ messages in thread
From: Ingo Schwarze @ 2010-10-24 16:40 UTC (permalink / raw)
  To: tech

Hi,

not only Xenocara (i.e. OpenBSD X.org), but various ports as well
use .so.  I'm not thrilled about that, i think .so is useful for
general-purpose typesetting, but rather dumb in a manual formatter.
Yet, being even less thrilled about the prospect to rewrite the
X.org build system or about patching lots of ports, i made up my
mind to implement .so.

As a first step, here is a patch to split pdesc (p = partial
as opposed to f = full) out of fdesc in main.c, such that
it can later be called when encountering .so.  Yes, i am aware
that Kristaps is about to refactor fdesc as well - but i consider
that all the more a reason to first push in what we need in
terms of featuritis...  ;-)

The patch looks bigger than it is, most of the code is just moved,
not changed.

Somehow, i *feel* like .so is a bad idea from a security perspective,
so i'm considering to add some additional checks, see below.
The man(1), and thus, mandoc(1) utilities are meant to be called
by privileged users, even on untrusted input data - no admin would
check the manuals of software installed on a server for malicious
content.  Even though root is allowed to read each file on the
system, he may not want a sensitive file displayed on the screen
just because a manual contains a malicious .so directive referring
to it.  Besides, some operating systems install man(1) SGID or even
SUID in order to do caching.  I hope we will never do this in
OpenBSD, but still, mandoc(1) is not OpenBSD-only, so i don't want
to expose other systems to any avoidable risks, either - like
symlink attacks.

I admit that, discussing this with naddy@, sthen@ and phessler@,
they all call me paranoid, and i am unable to provide really
compelling exploits based on .so.  Besides, OpenBSD groff always
supported .so, and it was called from man.conf, though not SGID.
Also, naddy@ has a point that people choosing to make man(1) SUID
will probably have worse problems than merely .so...
Still, an uneasy feeling remains about it.

Thoughts - both about the refactoring patch below
and about .so security implications?

Yours,
  Ingo

----- Forwarded message from Ingo Schwarze <schwarze@usta.de> -----

From: Ingo Schwarze <schwarze@usta.de>
Date: Sun, 24 Oct 2010 13:48:41 +0200
To: matthieu@
Cc: todd@, espie@, naddy@
Subject: Re: mandoc and x, we good?

Hi Matthieu,

> - For the .so, it seems to me that adding .so support to mandoc should
>   not be too hard to implement. 

After thinking a bit more and discussing with espie@ and a bit with
naddy@, i'm now starting to implement .so for mandoc, very probably
taking the following precautions:

 1. In case the given path does not resolve to a regular file,
    skip the .so directive and throw a non-fatal ERROR.

 2. In case you are running issetugid(2),
    skip the .so directive and throw a non-fatal ERROR.

 3. In case you are running geteuid(2) root,
    drop privilege to a new _mandoc user before reading the file,
    and resume privilege after reading the file, such that you can
    process additional private files specified on the command line.

Thus, i hope we can avoid useless effort reorganizing xenocara
manuals.  Additionally, various ports appear to use .so, too.

Yours,
  Ingo

----- End forwarded message -----

Index: main.c
===================================================================
RCS file: /cvs/src/usr.bin/mandoc/main.c,v
retrieving revision 1.49
diff -u -p -r1.49 main.c
--- main.c	16 Oct 2010 20:49:37 -0000	1.49
+++ main.c	24 Oct 2010 16:06:18 -0000
@@ -179,6 +179,7 @@ static	const char * const	mandocerrs[MAN
 	"static buffer exhausted",
 };
 
+static	void		  pdesc(struct curparse *);
 static	void		  fdesc(struct curparse *);
 static	void		  ffile(const char *, struct curparse *);
 static	int		  moptions(enum intt *, char *);
@@ -392,6 +393,124 @@ read_whole_file(struct curparse *curp, s
 static void
 fdesc(struct curparse *curp)
 {
+	struct man	*man;
+	struct mdoc	*mdoc;
+	struct roff	*roff;
+
+	pdesc(curp);
+
+	man  = curp->man;
+	mdoc = curp->mdoc;
+	roff = curp->roff;
+
+	if (MANDOCLEVEL_FATAL <= exit_status)
+		goto cleanup;
+
+	/* NOTE a parser may not have been assigned, yet. */
+
+	if ( ! (man || mdoc)) {
+		fprintf(stderr, "%s: Not a manual\n", curp->file);
+		exit_status = MANDOCLEVEL_FATAL;
+		goto cleanup;
+	}
+
+	/* Clean up the parse routine ASTs. */
+
+	if (mdoc && ! mdoc_endparse(mdoc)) {
+		assert(MANDOCLEVEL_FATAL <= exit_status);
+		goto cleanup;
+	}
+	if (man && ! man_endparse(man)) {
+		assert(MANDOCLEVEL_FATAL <= exit_status);
+		goto cleanup;
+	}
+	if (roff && ! roff_endparse(roff)) {
+		assert(MANDOCLEVEL_FATAL <= exit_status);
+		goto cleanup;
+	}
+
+	/*
+	 * With -Wstop and warnings or errors of at least
+	 * the requested level, do not produce output.
+	 */
+
+	if (MANDOCLEVEL_OK != exit_status && curp->wstop)
+		goto cleanup;
+
+	/* If unset, allocate output dev now (if applicable). */
+
+	if ( ! (curp->outman && curp->outmdoc)) {
+		switch (curp->outtype) {
+		case (OUTT_XHTML):
+			curp->outdata = xhtml_alloc(curp->outopts);
+			break;
+		case (OUTT_HTML):
+			curp->outdata = html_alloc(curp->outopts);
+			break;
+		case (OUTT_ASCII):
+			curp->outdata = ascii_alloc(curp->outopts);
+			curp->outfree = ascii_free;
+			break;
+		case (OUTT_PDF):
+			curp->outdata = pdf_alloc(curp->outopts);
+			curp->outfree = pspdf_free;
+			break;
+		case (OUTT_PS):
+			curp->outdata = ps_alloc(curp->outopts);
+			curp->outfree = pspdf_free;
+			break;
+		default:
+			break;
+		}
+
+		switch (curp->outtype) {
+		case (OUTT_HTML):
+			/* FALLTHROUGH */
+		case (OUTT_XHTML):
+			curp->outman = html_man;
+			curp->outmdoc = html_mdoc;
+			curp->outfree = html_free;
+			break;
+		case (OUTT_TREE):
+			curp->outman = tree_man;
+			curp->outmdoc = tree_mdoc;
+			break;
+		case (OUTT_PDF):
+			/* FALLTHROUGH */
+		case (OUTT_ASCII):
+			/* FALLTHROUGH */
+		case (OUTT_PS):
+			curp->outman = terminal_man;
+			curp->outmdoc = terminal_mdoc;
+			break;
+		default:
+			break;
+		}
+	}
+
+	/* Execute the out device, if it exists. */
+
+	if (man && curp->outman)
+		(*curp->outman)(curp->outdata, man);
+	if (mdoc && curp->outmdoc)
+		(*curp->outmdoc)(curp->outdata, mdoc);
+
+ cleanup:
+	memset(&curp->regs, 0, sizeof(struct regset));
+	if (mdoc)
+		mdoc_reset(mdoc);
+	if (man)
+		man_reset(man);
+	if (roff)
+		roff_reset(roff);
+
+	return;
+}
+
+
+static void
+pdesc(struct curparse *curp)
+{
 	struct buf	 ln, blk;
 	int		 i, pos, lnn, lnn_start, with_mmap, of;
 	enum rofferr	 re;
@@ -400,10 +519,6 @@ fdesc(struct curparse *curp)
 	struct mdoc	*mdoc;
 	struct roff	*roff;
 
-	man = NULL;
-	mdoc = NULL;
-	roff = NULL;
-
 	memset(&ln, 0, sizeof(struct buf));
 
 	/*
@@ -420,6 +535,8 @@ fdesc(struct curparse *curp)
 		curp->roff = roff_alloc(&curp->regs, curp, mmsg);
 	assert(curp->roff);
 	roff = curp->roff;
+	mdoc = curp->mdoc;
+	man  = curp->man;
 
 	for (i = 0, lnn = 1; i < (int)blk.sz;) {
 		pos = 0;
@@ -512,7 +629,7 @@ fdesc(struct curparse *curp)
 			continue;
 		} else if (ROFF_ERR == re) {
 			assert(MANDOCLEVEL_FATAL <= exit_status);
-			goto cleanup;
+			break;
 		}
 
 		/*
@@ -529,119 +646,19 @@ fdesc(struct curparse *curp)
 
 		if (man && ! man_parseln(man, lnn_start, ln.buf, of)) {
 			assert(MANDOCLEVEL_FATAL <= exit_status);
-			goto cleanup;
+			break;
 		}
 		if (mdoc && ! mdoc_parseln(mdoc, lnn_start, ln.buf, of)) {
 			assert(MANDOCLEVEL_FATAL <= exit_status);
-			goto cleanup;
-		}
-	}
-
-	/* NOTE a parser may not have been assigned, yet. */
-
-	if ( ! (man || mdoc)) {
-		fprintf(stderr, "%s: Not a manual\n", curp->file);
-		exit_status = MANDOCLEVEL_FATAL;
-		goto cleanup;
-	}
-
-	/* Clean up the parse routine ASTs. */
-
-	if (mdoc && ! mdoc_endparse(mdoc)) {
-		assert(MANDOCLEVEL_FATAL <= exit_status);
-		goto cleanup;
-	}
-	if (man && ! man_endparse(man)) {
-		assert(MANDOCLEVEL_FATAL <= exit_status);
-		goto cleanup;
-	}
-	if (roff && ! roff_endparse(roff)) {
-		assert(MANDOCLEVEL_FATAL <= exit_status);
-		goto cleanup;
-	}
-
-	/*
-	 * With -Wstop and warnings or errors of at least
-	 * the requested level, do not produce output.
-	 */
-
-	if (MANDOCLEVEL_OK != exit_status && curp->wstop)
-		goto cleanup;
-
-	/* If unset, allocate output dev now (if applicable). */
-
-	if ( ! (curp->outman && curp->outmdoc)) {
-		switch (curp->outtype) {
-		case (OUTT_XHTML):
-			curp->outdata = xhtml_alloc(curp->outopts);
-			break;
-		case (OUTT_HTML):
-			curp->outdata = html_alloc(curp->outopts);
-			break;
-		case (OUTT_ASCII):
-			curp->outdata = ascii_alloc(curp->outopts);
-			curp->outfree = ascii_free;
-			break;
-		case (OUTT_PDF):
-			curp->outdata = pdf_alloc(curp->outopts);
-			curp->outfree = pspdf_free;
-			break;
-		case (OUTT_PS):
-			curp->outdata = ps_alloc(curp->outopts);
-			curp->outfree = pspdf_free;
-			break;
-		default:
-			break;
-		}
-
-		switch (curp->outtype) {
-		case (OUTT_HTML):
-			/* FALLTHROUGH */
-		case (OUTT_XHTML):
-			curp->outman = html_man;
-			curp->outmdoc = html_mdoc;
-			curp->outfree = html_free;
-			break;
-		case (OUTT_TREE):
-			curp->outman = tree_man;
-			curp->outmdoc = tree_mdoc;
-			break;
-		case (OUTT_PDF):
-			/* FALLTHROUGH */
-		case (OUTT_ASCII):
-			/* FALLTHROUGH */
-		case (OUTT_PS):
-			curp->outman = terminal_man;
-			curp->outmdoc = terminal_mdoc;
-			break;
-		default:
 			break;
 		}
 	}
 
-	/* Execute the out device, if it exists. */
-
-	if (man && curp->outman)
-		(*curp->outman)(curp->outdata, man);
-	if (mdoc && curp->outmdoc)
-		(*curp->outmdoc)(curp->outdata, mdoc);
-
- cleanup:
-	memset(&curp->regs, 0, sizeof(struct regset));
-	if (mdoc)
-		mdoc_reset(mdoc);
-	if (man)
-		man_reset(man);
-	if (roff)
-		roff_reset(roff);
-	if (ln.buf)
-		free(ln.buf);
+	free(ln.buf);
 	if (with_mmap)
 		munmap(blk.buf, blk.sz);
 	else
 		free(blk.buf);
-
-	return;
 }
 
 
--
 To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: implement .so
  2010-10-24 16:40 implement .so Ingo Schwarze
@ 2010-10-24 16:49 ` Joerg Sonnenberger
  2010-10-24 17:29   ` Ingo Schwarze
  0 siblings, 1 reply; 19+ messages in thread
From: Joerg Sonnenberger @ 2010-10-24 16:49 UTC (permalink / raw)
  To: tech

On Sun, Oct 24, 2010 at 06:40:57PM +0200, Ingo Schwarze wrote:
> not only Xenocara (i.e. OpenBSD X.org), but various ports as well
> use .so.  I'm not thrilled about that, i think .so is useful for
> general-purpose typesetting, but rather dumb in a manual formatter.
> Yet, being even less thrilled about the prospect to rewrite the
> X.org build system or about patching lots of ports, i made up my
> mind to implement .so.

I don't think any patching itself has to be done. There is already a
tool called soelim to expand them statically. Remember that we discussed
.so in Rostock? The behavior is not exactly sane. So from a maintainance
perspective, I think it is much easier to just handle the 99% of easy
cases in ports/pkgsrc and patch the few remaining cases.

Joerg
--
 To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: implement .so
  2010-10-24 16:49 ` Joerg Sonnenberger
@ 2010-10-24 17:29   ` Ingo Schwarze
  2010-10-24 17:38     ` Joerg Sonnenberger
  0 siblings, 1 reply; 19+ messages in thread
From: Ingo Schwarze @ 2010-10-24 17:29 UTC (permalink / raw)
  To: tech

Hi Joerg,

Joerg Sonnenberger wrote on Sun, Oct 24, 2010 at 06:49:45PM +0200:
> On Sun, Oct 24, 2010 at 06:40:57PM +0200, Ingo Schwarze wrote:

>> not only Xenocara (i.e. OpenBSD X.org), but various ports as well
>> use .so.  I'm not thrilled about that, i think .so is useful for
>> general-purpose typesetting, but rather dumb in a manual formatter.
>> Yet, being even less thrilled about the prospect to rewrite the
>> X.org build system or about patching lots of ports, i made up my
>> mind to implement .so.

> I don't think any patching itself has to be done.
> There is already a tool called soelim to expand them statically.

I fear we can't use that.
We are planning to install unformatted manuals in the future
because mandoc is so fast that it can format the manuals on
the fly, even on the VAX.
Having the source has quite a few advantages.

Installing soelim and running that in a pipe with mandoc
does not look sane.

Running soelim at build time and installing multiple copies
of the same page looks even less sane.
That would be waste of space, bandwidth...

Converting .so to hard links would be an option if we were talking
about just one build system.  But ports have all kinds of different
build systems.

> Remember that we discussed .so in Rostock?
> The behavior is not exactly sane.

Yes, i don't say i like it.

> So from a maintainance perspective, I think it is much easier to
> just handle the 99% of easy cases in ports/pkgsrc and patch the
> few remaining cases.

Hm.
I don't see how that could work.

Yours,
  Ingo
--
 To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: implement .so
  2010-10-24 17:29   ` Ingo Schwarze
@ 2010-10-24 17:38     ` Joerg Sonnenberger
  2010-10-24 18:00       ` Ingo Schwarze
  0 siblings, 1 reply; 19+ messages in thread
From: Joerg Sonnenberger @ 2010-10-24 17:38 UTC (permalink / raw)
  To: tech

On Sun, Oct 24, 2010 at 07:29:14PM +0200, Ingo Schwarze wrote:
> Converting .so to hard links would be an option if we were talking
> about just one build system.  But ports have all kinds of different
> build systems.

Actually, if you consider ports itself as build system it is exactly
that.  Once you have run the "make install" of the target package,
iterate over the PLIST of the package and check for .so usage in the man
pages and fix them up. That's similar to fixing up compressed vs
uncompressed man pages.

Joerg
--
 To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: implement .so
  2010-10-24 17:38     ` Joerg Sonnenberger
@ 2010-10-24 18:00       ` Ingo Schwarze
  2010-10-24 18:15         ` Joerg Sonnenberger
  0 siblings, 1 reply; 19+ messages in thread
From: Ingo Schwarze @ 2010-10-24 18:00 UTC (permalink / raw)
  To: tech

Hi Joerg,

Joerg Sonnenberger wrote on Sun, Oct 24, 2010 at 07:38:57PM +0200:
> On Sun, Oct 24, 2010 at 07:29:14PM +0200, Ingo Schwarze wrote:

>> Converting .so to hard links would be an option if we were talking
>> about just one build system.  But ports have all kinds of different
>> build systems.

> Actually, if you consider ports itself as build system it is exactly
> that.  Once you have run the "make install" of the target package,
> iterate over the PLIST of the package and check for .so usage in the man
> pages and fix them up. That's similar to fixing up compressed vs
> uncompressed man pages.

That does indeed sound possible.

But i doubt it causes less maintenance work compared to just
implementing .so.

 * It would need to be done for both ports and Xenocara.
 * It would need to be done in each operating and ports system
   using mandoc; on a medium term, that may amount to at least
   four systems (OpenBSD, NetBSD, FreeBSD, DragonFly BSD).
 * From a maintenance perspective, .so does not seem that bad:
   it's strictly local in main.c without tentacles into any libs.
 * It would also put the code where it belongs: .so is a roff
   feature, not a build system feature.
 * All people i'm asking tend to say i should not worry that
   much about the my security concerns, maybe i'm indeed
   excessively paranoid in that respect.

Yours,
  Ingo
--
 To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: implement .so
  2010-10-24 18:00       ` Ingo Schwarze
@ 2010-10-24 18:15         ` Joerg Sonnenberger
  2010-10-24 18:17           ` Joerg Sonnenberger
  2010-10-24 19:41           ` Ingo Schwarze
  0 siblings, 2 replies; 19+ messages in thread
From: Joerg Sonnenberger @ 2010-10-24 18:15 UTC (permalink / raw)
  To: tech

On Sun, Oct 24, 2010 at 08:00:19PM +0200, Ingo Schwarze wrote:
>  * It would need to be done for both ports and Xenocara.
>  * It would need to be done in each operating and ports system
>    using mandoc; on a medium term, that may amount to at least
>    four systems (OpenBSD, NetBSD, FreeBSD, DragonFly BSD).

Having a single script like soelim (or whatever you want to call it)
minimizes the redundancy. It should be trivial to hook up in any of the
above systems. Attached is an AWK script that prints the link target to
stdout if it unique or returns failure otherwise. Shouldn't be hard to
polish so that it takes a list of files in stdin and does the magic,
potentially including things like using zcat etc.

>  * From a maintenance perspective, .so does not seem that bad:
>    it's strictly local in main.c without tentacles into any libs.
>  * It would also put the code where it belongs: .so is a roff
>    feature, not a build system feature.

Actually, there are a number of useful cases where you have to fix it up
already. Consider using compressed man pages by default. That breaks
badly with .so.

>  * All people i'm asking tend to say i should not worry that
>    much about the my security concerns, maybe i'm indeed
>    excessively paranoid in that respect.

Allow path names relative to the current directory and with /../ in
them.

Joerg
--
 To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: implement .so
  2010-10-24 18:15         ` Joerg Sonnenberger
@ 2010-10-24 18:17           ` Joerg Sonnenberger
  2010-10-24 19:41           ` Ingo Schwarze
  1 sibling, 0 replies; 19+ messages in thread
From: Joerg Sonnenberger @ 2010-10-24 18:17 UTC (permalink / raw)
  To: tech

[-- Attachment #1: Type: text/plain, Size: 12 bytes --]

Attachment.

[-- Attachment #2: soelim.awk --]
[-- Type: text/plain, Size: 113 bytes --]

#!/usr/bin/awk -f
/\.\\"/ { next }
/\.so/ {
	if (dst != "") exit 1
	dst=$2
	next
}
{ exit 1 }
END {
	print dst
}

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: implement .so
  2010-10-24 18:15         ` Joerg Sonnenberger
  2010-10-24 18:17           ` Joerg Sonnenberger
@ 2010-10-24 19:41           ` Ingo Schwarze
  2010-10-24 19:51             ` Joerg Sonnenberger
  1 sibling, 1 reply; 19+ messages in thread
From: Ingo Schwarze @ 2010-10-24 19:41 UTC (permalink / raw)
  To: tech

Hi Joerg,

Joerg Sonnenberger wrote on Sun, Oct 24, 2010 at 08:15:02PM +0200:
> On Sun, Oct 24, 2010 at 08:00:19PM +0200, Ingo Schwarze wrote:

>>  * It would need to be done for both ports and Xenocara.
>>  * It would need to be done in each operating and ports system
>>    using mandoc; on a medium term, that may amount to at least
>>    four systems (OpenBSD, NetBSD, FreeBSD, DragonFly BSD).

> Having a single script like soelim (or whatever you want to call it)
> minimizes the redundancy. It should be trivial to hook up in any of the
> above systems. Attached is an AWK script that prints the link target to
> stdout if it unique or returns failure otherwise. Shouldn't be hard to
> polish so that it takes a list of files in stdin and does the magic,
> potentially including things like using zcat etc.

I just talked to espie@ (who is maintaining our ports system)
and he confirmed that a solution of this kind would not be difficult
in our ports system.  But he does consider it clunky and would
prefer having a mandoc .so implementation.  In addition to my
remark that .so is rather a roff than a build system feature,
he said "Why rely on a partial workaround in the wrong place
when a full solution in the right place is easy to implement?"
Indeed, i'm not expecting .so to require much code.

Besides, Marc made the point that if we want to convince people
to accept mandoc as a standard implementation for formatting
manuals, we will almost certainly need .so support anyway:
Not everyone will agree that is useless, if even among ourselves
we are not 100% sure, so we risk fruitless and distracting
discussions whether mandoc is complete or defective.

Regarding Xenocara, matthieu@ obviously strongly prefers a mandoc
solution to a solution in the build system as well.  If i understand
correctly, that is in particular because the X.org build system is
already rather entangled and every additional bit of complexity adds
to the pain.

So, i'm definitely willing to get those discussions off the table
by just implementing the feature myself; provided the code will be
small, clean and local (and maintained by me), would you still be
opposed in principle to putting it in?

>>  * From a maintenance perspective, .so does not seem that bad:
>>    it's strictly local in main.c without tentacles into any libs.
>>  * It would also put the code where it belongs: .so is a roff
>>    feature, not a build system feature.
 
> Actually, there are a number of useful cases where you have to fix it
> up already.  Consider using compressed man pages by default.
> That breaks badly with .so.

Indeed, i don't expect we want to link against zlib.
So, such special needs would still need to be handled outside
mandoc - but only by those systems wanting them.

>>  * All people i'm asking tend to say i should not worry that
>>    much about the my security concerns, maybe i'm indeed
>>    excessively paranoid in that respect.

> Allow path names relative to the current directory and with /../
> in them.

Hm?  Sorry?
I definitely need to allow path names relative to the current dir.
That's used all over the place.

You mean, i should *not* allow absolute path names,
and i should not allow pathnames with ../ in them?

Yours,
  Ingo
--
 To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: implement .so
  2010-10-24 19:41           ` Ingo Schwarze
@ 2010-10-24 19:51             ` Joerg Sonnenberger
  2010-10-25 21:55               ` Ingo Schwarze
  0 siblings, 1 reply; 19+ messages in thread
From: Joerg Sonnenberger @ 2010-10-24 19:51 UTC (permalink / raw)
  To: tech

On Sun, Oct 24, 2010 at 09:41:30PM +0200, Ingo Schwarze wrote:
> Besides, Marc made the point that if we want to convince people
> to accept mandoc as a standard implementation for formatting
> manuals, we will almost certainly need .so support anyway:
> Not everyone will agree that is useless, if even among ourselves
> we are not 100% sure, so we risk fruitless and distracting
> discussions whether mandoc is complete or defective.

As we have seen in Rostock, full .so implementation has some funny side
effects, so I consider it a broken feature. I won't oppose a clean
patch, we have already seen that it is straight forward to implement.

> > Actually, there are a number of useful cases where you have to fix it
> > up already.  Consider using compressed man pages by default.
> > That breaks badly with .so.
> 
> Indeed, i don't expect we want to link against zlib.
> So, such special needs would still need to be handled outside
> mandoc - but only by those systems wanting them.

My point is that it essentially falls apart if you want to use
compressed man pages. So it really, really shouldn't be used.

> >>  * All people i'm asking tend to say i should not worry that
> >>    much about the my security concerns, maybe i'm indeed
> >>    excessively paranoid in that respect.
> 
> > Allow path names relative to the current directory and with /../
> > in them.
> 
> Hm?  Sorry?
> I definitely need to allow path names relative to the current dir.
> That's used all over the place.
> 
> You mean, i should *not* allow absolute path names,
> and i should not allow pathnames with ../ in them?

Sorry, yes. Only allow relative path names without /../ components.

Joerg
--
 To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: implement .so
  2010-10-24 19:51             ` Joerg Sonnenberger
@ 2010-10-25 21:55               ` Ingo Schwarze
  2010-10-25 22:10                 ` Joerg Sonnenberger
  0 siblings, 1 reply; 19+ messages in thread
From: Ingo Schwarze @ 2010-10-25 21:55 UTC (permalink / raw)
  To: tech

Hi Joerg,

Joerg Sonnenberger wrote on Sun, Oct 24, 2010 at 09:51:35PM +0200:

> As we have seen in Rostock, full .so implementation has some funny side
> effects, so I consider it a broken feature.

Yes, no doubt.  It is broken by design in multiple ways.

> I won't oppose a clean patch, we have already seen that it is
> straight forward to implement.

See below, feedback is welcome.
After the first round of review and polishing here
i'm planning to commit it to OpenBSD and have it looked
over by a few people round here.

> My point is that it essentially falls apart if you want to use
> compressed man pages.

Sure, i understand that.

> So it really, really shouldn't be used.

I fully agree.
This is to deal with existing stuff
and should not be used for new projects.

> Sorry, yes. Only allow relative path names without /../ components.

Actually, it is even worse.
All ports in the OpenBSD ports tree using the feature give the path
relative to the *parent* directory of the directory containing the
manual page having the .so directive.

So these are neither absolute paths (relative to the root of the
file system) nor relative paths (relative to the current working
directory).

What i'm doing for now is:

 * regarding validation of the .so directive (roff.c, roff_so()):
    - path must start with "man"
    - path must contain exactly one '/'
    - the characters before and after the slash must not be '.'
    - the slash must not be the last character
 * regarding validation of the path to the page containing
   the .so directive (main.c, pfile()):
    - path must contain at least one '/'
    - component before the slash must start with "man"

In this case, i replace the last two components of the path to
the page containing the .so directive by the contents of the
directive, or else error out.

Note that the patch builds upon the preceding refactoring patch
i sent yesterday, both building on the OpenBSD tree.

Yours,
  Ingo


diff -Napur mandoc.pdesc/main.c mandoc/main.c
--- mandoc.pdesc/main.c	Sun Oct 24 13:47:06 2010
+++ mandoc/main.c	Mon Oct 25 15:02:52 2010
@@ -15,6 +15,7 @@
  * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
  * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
+#include <sys/param.h>
 #include <sys/types.h>
 #include <sys/mman.h>
 #include <sys/stat.h>
@@ -173,6 +174,8 @@ static	const char * const	mandocerrs[MANDOCERR_MAX] = 
 	"argument count wrong, violates syntax",
 	"child violates parent syntax",
 	"argument count wrong, violates syntax",
+	"invalid path in include directive",
+	"include used by a file at an invalid path",
 	"no document body",
 	"no document prologue",
 	"static buffer exhausted",
@@ -181,6 +184,7 @@ static	const char * const	mandocerrs[MANDOCERR_MAX] = 
 static	void		  pdesc(struct curparse *);
 static	void		  fdesc(struct curparse *);
 static	void		  ffile(const char *, struct curparse *);
+static	int		  pfile(const char *, struct curparse *, int);
 static	int		  moptions(enum intt *, char *);
 static	int		  mmsg(enum mandocerr, void *, 
 				int, int, const char *);
@@ -306,7 +310,58 @@ ffile(const char *file, struct curparse *curp)
 		perror(curp->file);
 }
 
+static int
+pfile(const char *file, struct curparse *curp, int ln)
+{
+	char		 path[MAXPATHLEN];
+	const char	*savefile;
+	char		*p;
+	int		 savefd;
 
+	if (strlcpy(path, curp->file, sizeof(path)) >= sizeof(path)) {
+		mmsg(MANDOCERR_MEM, curp, ln, 1, NULL);
+		return(0);
+	}
+
+	if (NULL == (p = strrchr(path, '/'))) {
+		mmsg(MANDOCERR_SOUSER, curp, ln, 1, NULL);
+		return(0);
+	}
+
+	while (path < p && '/' != p[-1])
+		p--;
+
+	if (strncmp(p, "man", 3)) {
+		mmsg(MANDOCERR_SOUSER, curp, ln, 1, NULL);
+		return(0);
+	}
+	*p = '\0';
+
+	if (strlcat(path, file, sizeof(path)) >= sizeof(path)) {
+		mmsg(MANDOCERR_MEM, curp, ln, 1, NULL);
+		return(0);
+	}
+
+	savefile = curp->file;
+	savefd = curp->fd;
+
+	curp->file = path;
+	if (-1 == (curp->fd = open(curp->file, O_RDONLY, 0))) {
+		perror(curp->file);
+		exit_status = MANDOCLEVEL_SYSERR;
+	} else {
+		pdesc(curp);
+		if (-1 == close(curp->fd))
+			perror(curp->file);
+	}
+
+	curp->file = savefile;
+	curp->fd = savefd;
+
+	return(MANDOCLEVEL_FATAL > exit_status ? 1 : 0);
+}
+
+
 static void
 resize_buf(struct buf *buf, size_t initial)
 {
@@ -629,6 +684,11 @@ pdesc(struct curparse *curp)
 		} else if (ROFF_ERR == re) {
 			assert(MANDOCLEVEL_FATAL <= exit_status);
 			break;
+		} else if (ROFF_SO == re) {
+			if (pfile(ln.buf + of, curp, lnn_start))
+				continue;
+			else
+				break;
 		}
 
 		/*
diff -Napur mandoc.pdesc/mandoc.h mandoc/mandoc.h
--- mandoc.pdesc/mandoc.h	Sun Oct 24 13:47:06 2010
+++ mandoc/mandoc.h	Mon Oct 25 14:57:22 2010
@@ -114,6 +114,8 @@ enum	mandocerr {
 	MANDOCERR_SYNTARGVCOUNT, /* argument count wrong, violates syntax */
 	MANDOCERR_SYNTCHILD, /* child violates parent syntax */
 	MANDOCERR_SYNTARGCOUNT, /* argument count wrong, violates syntax */
+	MANDOCERR_SOPATH, /* invalid path in include directive */
+	MANDOCERR_SOUSER, /* include used by a file at an invalid path */
 	MANDOCERR_NODOCBODY, /* no document body */
 	MANDOCERR_NODOCPROLOG, /* no document prologue */
 	MANDOCERR_MEM, /* static buffer exhausted */
diff -Napur mandoc.pdesc/roff.c mandoc/roff.c
--- mandoc.pdesc/roff.c	Wed Oct 20 09:22:11 2010
+++ mandoc/roff.c	Mon Oct 25 14:38:24 2010
@@ -48,11 +48,12 @@ enum	rofft {
 	ROFF_ie,
 	ROFF_if,
 	ROFF_ig,
+	ROFF_nr,
 	ROFF_rm,
+	ROFF_so,
 	ROFF_tr,
 	ROFF_cblock,
 	ROFF_ccond, /* FIXME: remove this. */
-	ROFF_nr,
 	ROFF_MAX
 };
 
@@ -128,6 +129,7 @@ static	int		 roff_res(struct roff *, 
 				char **, size_t *, int);
 static	void		 roff_setstr(struct roff *,
 				const char *, const char *);
+static	enum rofferr	 roff_so(ROFF_ARGS);
 static	char		*roff_strdup(const char *);
 
 /* See roff_hash_find() */
@@ -150,11 +152,12 @@ static	struct roffmac	 roffs[ROFF_MAX] = {
 	{ "ie", roff_cond, roff_cond_text, roff_cond_sub, ROFFMAC_STRUCT, NULL },
 	{ "if", roff_cond, roff_cond_text, roff_cond_sub, ROFFMAC_STRUCT, NULL },
 	{ "ig", roff_block, roff_block_text, roff_block_sub, 0, NULL },
+	{ "nr", roff_nr, NULL, NULL, 0, NULL },
 	{ "rm", roff_line, NULL, NULL, 0, NULL },
+	{ "so", roff_so, NULL, NULL, 0, NULL },
 	{ "tr", roff_line, NULL, NULL, 0, NULL },
 	{ ".", roff_cblock, NULL, NULL, 0, NULL },
 	{ "\\}", roff_ccond, NULL, NULL, 0, NULL },
-	{ "nr", roff_nr, NULL, NULL, 0, NULL },
 };
 
 static	void		 roff_free1(struct roff *);
@@ -1005,6 +1008,26 @@ roff_nr(ROFF_ARGS)
 	}
 
 	return(ROFF_IGN);
+}
+
+
+/* ARGSUSED */
+static enum rofferr
+roff_so(ROFF_ARGS)
+{
+	char *name, *p;
+
+	name = *bufp + pos;
+	if (strncmp(name, "man", 3) ||
+	    NULL == (p = strchr(name, '/')) ||
+	    '.' == p[-1] || '.' == p[1] || '\0' == p[1] ||
+	    strchr(p+1, '/')) {
+		(*r->msg)(MANDOCERR_SOPATH, r->data, ln, pos, NULL);
+		return(ROFF_ERR);
+	}
+
+	*offs = pos;
+	return(ROFF_SO);
 }
 
 
diff -Napur mandoc.pdesc/roff.h mandoc/roff.h
--- mandoc.pdesc/roff.h	Wed Oct 20 09:22:11 2010
+++ mandoc/roff.h	Mon Oct 25 03:48:01 2010
@@ -20,6 +20,7 @@
 enum	rofferr {
 	ROFF_CONT, /* continue processing line */
 	ROFF_RERUN, /* re-run roff interpreter with offset */
+	ROFF_SO, /* include another file */
 	ROFF_IGN, /* ignore current line */
 	ROFF_ERR /* badness: puke and stop */
 };
--
 To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: implement .so
  2010-10-25 21:55               ` Ingo Schwarze
@ 2010-10-25 22:10                 ` Joerg Sonnenberger
  2010-10-26 17:59                   ` Ingo Schwarze
  0 siblings, 1 reply; 19+ messages in thread
From: Joerg Sonnenberger @ 2010-10-25 22:10 UTC (permalink / raw)
  To: tech

On Mon, Oct 25, 2010 at 11:55:06PM +0200, Ingo Schwarze wrote:
> > Sorry, yes. Only allow relative path names without /../ components.
> 
> Actually, it is even worse.
> All ports in the OpenBSD ports tree using the feature give the path
> relative to the *parent* directory of the directory containing the
> manual page having the .so directive.
> 
> So these are neither absolute paths (relative to the root of the
> file system) nor relative paths (relative to the current working
> directory).

The current working directory for man is /usr/share/man etc. and that's
inherited by the renderer.

Joerg
--
 To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: implement .so
  2010-10-25 22:10                 ` Joerg Sonnenberger
@ 2010-10-26 17:59                   ` Ingo Schwarze
  2010-10-26 18:12                     ` Joerg Sonnenberger
  0 siblings, 1 reply; 19+ messages in thread
From: Ingo Schwarze @ 2010-10-26 17:59 UTC (permalink / raw)
  To: tech

Hi Joerg,

Joerg Sonnenberger wrote on Tue, Oct 26, 2010 at 12:10:17AM +0200:
> On Mon, Oct 25, 2010 at 11:55:06PM +0200, Ingo Schwarze wrote:

>> Actually, it is even worse.
>> All ports in the OpenBSD ports tree using the feature give the path
>> relative to the *parent* directory of the directory containing the
>> manual page having the .so directive.
>> 
>> So these are neither absolute paths (relative to the root of the
>> file system) nor relative paths (relative to the current working
>> directory).

> The current working directory for man is /usr/share/man etc. and that's
> inherited by the renderer.

Um.  Right.

While what i did is minimally more flexible - you can then call mandoc
from anywhere, as long as the source file is in the right place -
that is probably not worth the extra complexity.

So, here is a simplified patch (less than 50 new lines of code,
all very simple) relying on the current working directory.


diff -Napur mandoc.pdesc/main.c mandoc/main.c
--- mandoc.pdesc/main.c	Sun Oct 24 13:47:06 2010
+++ mandoc/main.c	Tue Oct 26 11:34:33 2010
@@ -173,6 +173,7 @@ static	const char * const	mandocerrs[MANDOCERR_MAX] = 
 	"argument count wrong, violates syntax",
 	"child violates parent syntax",
 	"argument count wrong, violates syntax",
+	"invalid path in include directive",
 	"no document body",
 	"no document prologue",
 	"static buffer exhausted",
@@ -181,6 +182,7 @@ static	const char * const	mandocerrs[MANDOCERR_MAX] = 
 static	void		  pdesc(struct curparse *);
 static	void		  fdesc(struct curparse *);
 static	void		  ffile(const char *, struct curparse *);
+static	int		  pfile(const char *, struct curparse *, int);
 static	int		  moptions(enum intt *, char *);
 static	int		  mmsg(enum mandocerr, void *, 
 				int, int, const char *);
@@ -306,7 +308,36 @@ ffile(const char *file, struct curparse *curp)
 		perror(curp->file);
 }
 
+static int
+pfile(const char *file, struct curparse *curp, int ln)
+{
+	const char	*savefile;
+	int		 fd, savefd;
 
+	if (-1 == (fd = open(file, O_RDONLY, 0))) {
+		perror(file);
+		exit_status = MANDOCLEVEL_SYSERR;
+		return(0);
+	}
+
+	savefile = curp->file;
+	savefd = curp->fd;
+
+	curp->file = file;
+	curp->fd = fd;
+
+	pdesc(curp);
+
+	curp->file = savefile;
+	curp->fd = savefd;
+
+	if (-1 == close(fd))
+		perror(file);
+
+	return(MANDOCLEVEL_FATAL > exit_status ? 1 : 0);
+}
+
+
 static void
 resize_buf(struct buf *buf, size_t initial)
 {
@@ -629,6 +660,11 @@ pdesc(struct curparse *curp)
 		} else if (ROFF_ERR == re) {
 			assert(MANDOCLEVEL_FATAL <= exit_status);
 			break;
+		} else if (ROFF_SO == re) {
+			if (pfile(ln.buf + of, curp, lnn_start))
+				continue;
+			else
+				break;
 		}
 
 		/*
diff -Napur mandoc.pdesc/mandoc.h mandoc/mandoc.h
--- mandoc.pdesc/mandoc.h	Sun Oct 24 13:47:06 2010
+++ mandoc/mandoc.h	Tue Oct 26 11:34:44 2010
@@ -114,6 +114,7 @@ enum	mandocerr {
 	MANDOCERR_SYNTARGVCOUNT, /* argument count wrong, violates syntax */
 	MANDOCERR_SYNTCHILD, /* child violates parent syntax */
 	MANDOCERR_SYNTARGCOUNT, /* argument count wrong, violates syntax */
+	MANDOCERR_SOPATH, /* invalid path in include directive */
 	MANDOCERR_NODOCBODY, /* no document body */
 	MANDOCERR_NODOCPROLOG, /* no document prologue */
 	MANDOCERR_MEM, /* static buffer exhausted */
diff -Napur mandoc.pdesc/roff.c mandoc/roff.c
--- mandoc.pdesc/roff.c	Wed Oct 20 09:22:11 2010
+++ mandoc/roff.c	Tue Oct 26 11:44:30 2010
@@ -48,11 +48,12 @@ enum	rofft {
 	ROFF_ie,
 	ROFF_if,
 	ROFF_ig,
+	ROFF_nr,
 	ROFF_rm,
+	ROFF_so,
 	ROFF_tr,
 	ROFF_cblock,
 	ROFF_ccond, /* FIXME: remove this. */
-	ROFF_nr,
 	ROFF_MAX
 };
 
@@ -128,6 +129,7 @@ static	int		 roff_res(struct roff *, 
 				char **, size_t *, int);
 static	void		 roff_setstr(struct roff *,
 				const char *, const char *);
+static	enum rofferr	 roff_so(ROFF_ARGS);
 static	char		*roff_strdup(const char *);
 
 /* See roff_hash_find() */
@@ -150,11 +152,12 @@ static	struct roffmac	 roffs[ROFF_MAX] = {
 	{ "ie", roff_cond, roff_cond_text, roff_cond_sub, ROFFMAC_STRUCT, NULL },
 	{ "if", roff_cond, roff_cond_text, roff_cond_sub, ROFFMAC_STRUCT, NULL },
 	{ "ig", roff_block, roff_block_text, roff_block_sub, 0, NULL },
+	{ "nr", roff_nr, NULL, NULL, 0, NULL },
 	{ "rm", roff_line, NULL, NULL, 0, NULL },
+	{ "so", roff_so, NULL, NULL, 0, NULL },
 	{ "tr", roff_line, NULL, NULL, 0, NULL },
 	{ ".", roff_cblock, NULL, NULL, 0, NULL },
 	{ "\\}", roff_ccond, NULL, NULL, 0, NULL },
-	{ "nr", roff_nr, NULL, NULL, 0, NULL },
 };
 
 static	void		 roff_free1(struct roff *);
@@ -1005,6 +1008,23 @@ roff_nr(ROFF_ARGS)
 	}
 
 	return(ROFF_IGN);
+}
+
+
+/* ARGSUSED */
+static enum rofferr
+roff_so(ROFF_ARGS)
+{
+	char *name;
+
+	name = *bufp + pos;
+	if ('/' == *name || strstr(name, "../") || strstr(name, "/..")) {
+		(*r->msg)(MANDOCERR_SOPATH, r->data, ln, pos, NULL);
+		return(ROFF_ERR);
+	}
+
+	*offs = pos;
+	return(ROFF_SO);
 }
 
 
diff -Napur mandoc.pdesc/roff.h mandoc/roff.h
--- mandoc.pdesc/roff.h	Wed Oct 20 09:22:11 2010
+++ mandoc/roff.h	Mon Oct 25 03:48:01 2010
@@ -20,6 +20,7 @@
 enum	rofferr {
 	ROFF_CONT, /* continue processing line */
 	ROFF_RERUN, /* re-run roff interpreter with offset */
+	ROFF_SO, /* include another file */
 	ROFF_IGN, /* ignore current line */
 	ROFF_ERR /* badness: puke and stop */
 };
--
 To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: implement .so
  2010-10-26 17:59                   ` Ingo Schwarze
@ 2010-10-26 18:12                     ` Joerg Sonnenberger
  2010-10-26 21:48                       ` Ingo Schwarze
  2010-10-26 23:10                       ` Ingo Schwarze
  0 siblings, 2 replies; 19+ messages in thread
From: Joerg Sonnenberger @ 2010-10-26 18:12 UTC (permalink / raw)
  To: tech

On Tue, Oct 26, 2010 at 07:59:13PM +0200, Ingo Schwarze wrote:
> So, here is a simplified patch (less than 50 new lines of code,
> all very simple) relying on the current working directory.

I would like to a see a warning for -T lint, but I am not sure how that
could be implemented easily.

> +roff_so(ROFF_ARGS)
> +{
> +	char *name;
> +
> +	name = *bufp + pos;
> +	if ('/' == *name || strstr(name, "../") || strstr(name, "/..")) {
> +		(*r->msg)(MANDOCERR_SOPATH, r->data, ln, pos, NULL);
> +		return(ROFF_ERR);
> +	}

Not sure I like the double use of strstr. What about searching for the
rare ".." first?

Joerg
--
 To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: implement .so
  2010-10-26 18:12                     ` Joerg Sonnenberger
@ 2010-10-26 21:48                       ` Ingo Schwarze
  2010-10-26 21:56                         ` Joerg Sonnenberger
  2010-10-26 23:10                       ` Ingo Schwarze
  1 sibling, 1 reply; 19+ messages in thread
From: Ingo Schwarze @ 2010-10-26 21:48 UTC (permalink / raw)
  To: tech

Hi Joerg,

Thanks for your quick feedback!

Joerg Sonnenberger wrote on Tue, Oct 26, 2010 at 08:12:52PM +0200:

> I would like to a see a warning for -T lint, but I am not sure how that
> could be implemented easily.

I think i know how to put that in, it doesn't seem like any kind of
a problem.  But i'd rather do that after committing the ERROR cleanup
i posted recently, or it will conflict badly.

> On Tue, Oct 26, 2010 at 07:59:13PM +0200, Ingo Schwarze wrote:

>> +roff_so(ROFF_ARGS)
>> +{
>> +	char *name;
>> +
>> +	name = *bufp + pos;
>> +	if ('/' == *name || strstr(name, "../") || strstr(name, "/..")) {
>> +		(*r->msg)(MANDOCERR_SOPATH, r->data, ln, pos, NULL);
>> +		return(ROFF_ERR);
>> +	}

> Not sure I like the double use of strstr.
> What about searching for the rare ".." first?

Admittedly, that would allow to reduce the algorithm order
from O(2N) to O(N), where N is the length of the name of the
included file, so not exactly a large number.

Given that you almost never have more than one .so per file,
i consider readability of the code more important here than
economizing typically about 20 character comparisons.

So, i think i will put in the pdesc refactoring, then .so,
then the ERROR cleanup, then add a warning message.

OK?

Yours,
  Ingo
--
 To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: implement .so
  2010-10-26 21:48                       ` Ingo Schwarze
@ 2010-10-26 21:56                         ` Joerg Sonnenberger
  0 siblings, 0 replies; 19+ messages in thread
From: Joerg Sonnenberger @ 2010-10-26 21:56 UTC (permalink / raw)
  To: tech

On Tue, Oct 26, 2010 at 11:48:38PM +0200, Ingo Schwarze wrote:
> > I would like to a see a warning for -T lint, but I am not sure how that
> > could be implemented easily.
> 
> I think i know how to put that in, it doesn't seem like any kind of
> a problem.  But i'd rather do that after committing the ERROR cleanup
> i posted recently, or it will conflict badly.

Fine with me.

> Admittedly, that would allow to reduce the algorithm order
> from O(2N) to O(N), where N is the length of the name of the
> included file, so not exactly a large number.

It's a bit more because strstr is doing an inner loop, so things like
/./././././. would be quite bad, but that's irrelvant. I want to bring
it up.

> OK?

Yes.

Joerg
--
 To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: implement .so
  2010-10-26 18:12                     ` Joerg Sonnenberger
  2010-10-26 21:48                       ` Ingo Schwarze
@ 2010-10-26 23:10                       ` Ingo Schwarze
  2010-10-26 23:30                         ` Joerg Sonnenberger
  1 sibling, 1 reply; 19+ messages in thread
From: Ingo Schwarze @ 2010-10-26 23:10 UTC (permalink / raw)
  To: tech

Hi Joerg,

Joerg Sonnenberger wrote on Tue, Oct 26, 2010 at 08:12:52PM +0200:

> I would like to a see a warning for -T lint, but I am not sure how that
> could be implemented easily.

Is this what you hoped for?

ischwarze@isnote $ mandoc -Tlint /usr/X11R6/man/man3/XevieStart.3 
/usr/X11R6/man/man3/XevieStart.3:1:1: WARNING: .so is fragile, better use ln(1)
man3/Xevie.3: No such file or directory
ischwarze@isnote $ cd /usr/X11R6/man/
ischwarze@isnote $ mandoc -Tlint man3/XevieStart.3
man3/XevieStart.3:1:1: WARNING: .so is fragile, better use ln(1)
man3/Xevie.3:10:13: WARNING: cannot parse date argument
man3/Xevie.3:16:7: WARNING: end of line whitespace
man3/Xevie.3:45:43: WARNING: end of line whitespace

Yours,
  Ingo


Index: main.c
===================================================================
RCS file: /cvs/src/usr.bin/mandoc/main.c,v
retrieving revision 1.53
diff -u -p -r1.53 main.c
--- main.c	26 Oct 2010 22:48:07 -0000	1.53
+++ main.c	26 Oct 2010 23:08:59 -0000
@@ -115,6 +115,7 @@ static	const char * const	mandocerrs[MAN
 	"macro not allowed in body",
 
 	/* related to document structure */
+	".so is fragile, better use ln(1)",
 	"NAME section must come first",
 	"bad NAME section contents",
 	"manual name not yet set",
Index: mandoc.h
===================================================================
RCS file: /cvs/src/usr.bin/mandoc/mandoc.h,v
retrieving revision 1.19
diff -u -p -r1.19 mandoc.h
--- mandoc.h	26 Oct 2010 22:48:07 -0000	1.19
+++ mandoc.h	26 Oct 2010 23:08:59 -0000
@@ -57,6 +57,7 @@ enum	mandocerr {
 	MANDOCERR_BADBODY, /* macro not allowed in body */
 
 	/* related to document structure */
+	MANDOCERR_SO, /* .so is fragile, better use ln(1) */
 	MANDOCERR_NAMESECFIRST, /* NAME section must come first */
 	MANDOCERR_BADNAMESEC, /* bad NAME section contents */
 	MANDOCERR_NONAME, /* manual name not yet set */
Index: roff.c
===================================================================
RCS file: /cvs/src/usr.bin/mandoc/roff.c,v
retrieving revision 1.14
diff -u -p -r1.14 roff.c
--- roff.c	26 Oct 2010 22:28:57 -0000	1.14
+++ roff.c	26 Oct 2010 23:08:59 -0000
@@ -1017,6 +1017,8 @@ roff_so(ROFF_ARGS)
 {
 	char *name;
 
+	(*r->msg)(MANDOCERR_SO, r->data, ln, ppos, NULL);
+
 	name = *bufp + pos;
 	if ('/' == *name || strstr(name, "../") || strstr(name, "/..")) {
 		(*r->msg)(MANDOCERR_SOPATH, r->data, ln, pos, NULL);
--
 To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: implement .so
  2010-10-26 23:10                       ` Ingo Schwarze
@ 2010-10-26 23:30                         ` Joerg Sonnenberger
  2010-10-26 23:41                           ` Ingo Schwarze
  0 siblings, 1 reply; 19+ messages in thread
From: Joerg Sonnenberger @ 2010-10-26 23:30 UTC (permalink / raw)
  To: tech

On Wed, Oct 27, 2010 at 01:10:56AM +0200, Ingo Schwarze wrote:
> Hi Joerg,
> 
> Joerg Sonnenberger wrote on Tue, Oct 26, 2010 at 08:12:52PM +0200:
> 
> > I would like to a see a warning for -T lint, but I am not sure how that
> > could be implemented easily.
> 
> Is this what you hoped for?
> 
> ischwarze@isnote $ mandoc -Tlint /usr/X11R6/man/man3/XevieStart.3 
> /usr/X11R6/man/man3/XevieStart.3:1:1: WARNING: .so is fragile, better use ln(1)
> man3/Xevie.3: No such file or directory
> ischwarze@isnote $ cd /usr/X11R6/man/
> ischwarze@isnote $ mandoc -Tlint man3/XevieStart.3
> man3/XevieStart.3:1:1: WARNING: .so is fragile, better use ln(1)

Yes.

Joerg
--
 To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: implement .so
  2010-10-26 23:30                         ` Joerg Sonnenberger
@ 2010-10-26 23:41                           ` Ingo Schwarze
  2010-10-27  9:34                             ` Kristaps Dzonsons
  0 siblings, 1 reply; 19+ messages in thread
From: Ingo Schwarze @ 2010-10-26 23:41 UTC (permalink / raw)
  To: tech

Hi Joerg,

Joerg Sonnenberger wrote on Wed, Oct 27, 2010 at 01:30:05AM +0200:
> On Wed, Oct 27, 2010 at 01:10:56AM +0200, Ingo Schwarze wrote:

>> man3/XevieStart.3:1:1: WARNING: .so is fragile, better use ln(1)

> Yes.

Thanks for checking, i have put it into OpenBSD.

We will have a bit of work resynching when Kristaps springs
back to life - but i think i will rather wait for his opinion
before uprooting his repo.  I think he uttered slightly different
views on at least one of the points i have been working on during
the last few days...

Yours,
  Ingo
--
 To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: implement .so
  2010-10-26 23:41                           ` Ingo Schwarze
@ 2010-10-27  9:34                             ` Kristaps Dzonsons
  0 siblings, 0 replies; 19+ messages in thread
From: Kristaps Dzonsons @ 2010-10-27  9:34 UTC (permalink / raw)
  To: tech

On 27/10/2010 01:41, Ingo Schwarze wrote:
> Hi Joerg,
>
> Joerg Sonnenberger wrote on Wed, Oct 27, 2010 at 01:30:05AM +0200:
>> On Wed, Oct 27, 2010 at 01:10:56AM +0200, Ingo Schwarze wrote:
>
>>> man3/XevieStart.3:1:1: WARNING: .so is fragile, better use ln(1)
>
>> Yes.
>
> Thanks for checking, i have put it into OpenBSD.
>
> We will have a bit of work resynching when Kristaps springs
> back to life - but i think i will rather wait for his opinion
> before uprooting his repo.  I think he uttered slightly different
> views on at least one of the points i have been working on during
> the last few days...

I'm getting there... ;-)

I don't think I've an issue with .so, but have barely had time to look 
at it.  I do think, however, I can come back with a patch and some 
cohesive arguments in favour of a different tbl approach.  More on this 
soon...

Kristaps
--
 To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2010-10-27  9:34 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-10-24 16:40 implement .so Ingo Schwarze
2010-10-24 16:49 ` Joerg Sonnenberger
2010-10-24 17:29   ` Ingo Schwarze
2010-10-24 17:38     ` Joerg Sonnenberger
2010-10-24 18:00       ` Ingo Schwarze
2010-10-24 18:15         ` Joerg Sonnenberger
2010-10-24 18:17           ` Joerg Sonnenberger
2010-10-24 19:41           ` Ingo Schwarze
2010-10-24 19:51             ` Joerg Sonnenberger
2010-10-25 21:55               ` Ingo Schwarze
2010-10-25 22:10                 ` Joerg Sonnenberger
2010-10-26 17:59                   ` Ingo Schwarze
2010-10-26 18:12                     ` Joerg Sonnenberger
2010-10-26 21:48                       ` Ingo Schwarze
2010-10-26 21:56                         ` Joerg Sonnenberger
2010-10-26 23:10                       ` Ingo Schwarze
2010-10-26 23:30                         ` Joerg Sonnenberger
2010-10-26 23:41                           ` Ingo Schwarze
2010-10-27  9:34                             ` Kristaps Dzonsons

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).