Re: mandocdb: do not use bogus files

[Kristaps Dzonsons <kristaps@bsd.lv>]

On 14/11/2011 20:13, Ingo Schwarze wrote:
> Hi,
>
> because this patch started getting large and unwieldy, and i'm trying
> to move fast, i have just put it into OpenBSD:
>
>   ----- 8<  ----- schnipp ----->8 ----- 8<  ----- schnapp ----->8 -----
>
> CVSROOT:        /cvs
> Module name:    src
> Changes by:     schwarze@cvs.openbsd.org        2011/11/14 11:52:05
>
> Modified files:
>          usr.bin/mandoc : mandocdb.8 mandocdb.c
>
> Log message:
> Store page titles in the correct case, and by default, only
> put stuff into the database that man(1) will be able to retrieve.
> However, support an option to use all directories and files.
>
> Kristaps@ agreed with the general direction and provided some feedback.
>
>   ----- 8<  ----- schnipp ----->8 ----- 8<  ----- schnapp ----->8 -----
>
> Of course, more tweaking will be required, and i'm open for
> suggestions!
>
> Tell me if you think i should merge to bsd.lv;
> for that purpose, i'm appending the committed patch below.

Hi Ingo,

This is a good start, but it needs more documentation as to what's 
happening.  You mention man(1) a great deal, but I can't easily find an 
authoritative list of man's behaviour in this regard (not on a 4.9 box, 
anyway).

> -.Op Fl v
> +.Op Fl av
>   .Op Ar dir...
>   .Nm
>   .Op Fl v
> @@ -42,8 +42,15 @@ manuals and indexes them in a
>   and
>   .Sx Index Database
>   for fast retrieval.
> +.Pp
>   The arguments are as follows:
>   .Bl -tag -width Ds
> +.It Fl a
> +Use all directories and files found below
> +.Ar dir ... .
> +By default, directories and files
> +.Xr man 1
> +cannot find will be silently skipped.

Can you instead list the measures taken and remove the Xr man reference? 
  This reference is not portable.

> +		/*
> +		 * Make sure the manual section and architecture
> +		 * agree with the directory where the file is located
> +		 * or man(1) will not be able to find it.
> +		 */

Same here.  Yes, this seems like quibbling, but without making this 
change all of these comments are useless beyond OpenBSD.  Plus, I'd 
rather have the correct measures in-line than having to infer behaviour.

>   	for (i = 0; i<  argc; i++) {
> +
> +		/*
> +		 * Analyze the path.
> +		 */

Hm.  Can be you be more specific?  Yes, I can figure it out by looking 
at the code (the strrchr() call threw me: I've never seen it used like 
that).  But I'd rather you describe it beforehand.

> +
> +		if (strlcpy(buf, argv[i], sizeof(buf))>= sizeof(buf)) {
> +			fprintf(stderr, "%s: Path too long\n", argv[i]);
> +			continue;
> +		}
> +		sec = arch = title = NULL;
> +		p = strrchr(buf, '\0');
> +		while (p-->  buf) {
> +			if (NULL == sec&&  '.' == *p) {
> +				sec = p + 1;
> +				*p = '\0';
> +				continue;
> +			}
> +			if ('/' != *p)
> +				continue;
> +			if (NULL == title) {
> +				title = p + 1;
> +				*p = '\0';
> +				continue;
> +			}
> +			if (strncmp("man", p + 1, 3))
> +				arch = p + 1;
> +			break;
> +		}
> +		if (NULL == title)
> +			title = buf;
> +
> +		/*
> +		 * Build the file structure.
> +		 */
> +
>   		nof = mandoc_calloc(1, sizeof(struct of));
> -		nof->fname = strdup(argv[i]);
> +		nof->fname = mandoc_strdup(argv[i]);
> +		if (NULL != sec)
> +			nof->sec = mandoc_strdup(sec);
> +		if (NULL != arch)
> +			nof->arch = mandoc_strdup(arch);
> +		nof->title = mandoc_strdup(title);
> +
> +		/*
> +		 * Add the structure to the list.
> +		 */
> +
>   		if (verb>  2)
>   			printf("%s: Scheduling\n", argv[i]);
>   		if (NULL == *of) {
> @@ -1180,12 +1275,14 @@ ofile_argbuild(char *argv[], int argc, i
>    * Pass in a pointer to a NULL structure for the first invocation.
>    */
>   static int
> -ofile_dirbuild(const char *dir, int verb, struct of **of)
> +ofile_dirbuild(const char *dir, const char* psec, const char *parch,
> +		int use_all, int verb, struct of **of)
>   {
>   	char		 buf[MAXPATHLEN];
>   	size_t		 sz;
>   	DIR		*d;
> -	const char	*fn;
> +	const char	*fn, *sec, *arch;
> +	char		*suffix;
>   	struct of	*nof;
>   	struct dirent	*dp;
>
> @@ -1196,10 +1293,30 @@ ofile_dirbuild(const char *dir, int verb
>
>   	while (NULL != (dp = readdir(d))) {
>   		fn = dp->d_name;
> +
> +		if ('.' == *fn)
> +			continue;
> +
>   		if (DT_DIR == dp->d_type) {
> -			if (0 == strcmp(".", fn))
> -				continue;
> -			if (0 == strcmp("..", fn))
> +			sec = psec;
> +			arch = parch;
> +
> +			/*
> +	 		 * Don't bother parsing directories
> +			 * that man(1) won't find.
> +			 */

Same argument as above....

> +
> +			if (NULL == sec) {
> +				if(0 == strncmp("man", fn, 3))
> +					sec = fn + 3;
> +				else if (use_all)
> +					sec = fn;
> +				else
> +					continue;
> +			} else if (NULL == arch&&  (use_all ||
> +					NULL == strchr(fn, '.')))
> +				arch = fn;
> +			else if (0 == use_all)
>   				continue;
>
>   			buf[0] = '\0';
> @@ -1207,22 +1324,35 @@ ofile_dirbuild(const char *dir, int verb
>   			strlcat(buf, "/", MAXPATHLEN);
>   			sz = strlcat(buf, fn, MAXPATHLEN);
>
> -			if (sz<  MAXPATHLEN) {
> -				if ( ! ofile_dirbuild(buf, verb, of))
> -					return(0);
> -				continue;
> -			} else if (sz<  MAXPATHLEN)
> -				continue;
> -
> -			fprintf(stderr, "%s: Path too long\n", dir);
> -			return(0);
> +			if (MAXPATHLEN<= sz) {
> +				fprintf(stderr, "%s: Path too long\n", dir);
> +				return(0);
> +			}
> +
> +			if (verb>  2)
> +				printf("%s: Scanning\n", buf);
> +
> +			if ( ! ofile_dirbuild(buf, sec, arch,
> +					use_all, verb, of))
> +				return(0);
>   		}
> -		if (DT_REG != dp->d_type)
> +		if (DT_REG != dp->d_type ||
> +		    (NULL == psec&&  !use_all) ||
> +		    !strcmp(MANDOC_DB, fn) ||
> +		    !strcmp(MANDOC_IDX, fn))
>   			continue;
>
> -		if (0 == strcmp(MANDOC_DB, fn) ||
> -				0 == strcmp(MANDOC_IDX, fn))
> -			continue;
> +		/*
> +		 * Don't bother parsing files that man(1) won't find.
> +		 */

Again...

> +
> +		suffix = strrchr(fn, '.');
> +		if (0 == use_all) {
> +			if (NULL == suffix)
> +				continue;
> +			if (strcmp(suffix + 1, psec))
> +				continue;
> +		}
>
>   		buf[0] = '\0';
>   		strlcat(buf, dir, MAXPATHLEN);
> @@ -1235,6 +1365,13 @@ ofile_dirbuild(const char *dir, int verb
>
>   		nof = mandoc_calloc(1, sizeof(struct of));
>   		nof->fname = mandoc_strdup(buf);
> +		if (NULL != psec)
> +			nof->sec = mandoc_strdup(psec);
> +		if (NULL != parch)
> +			nof->arch = mandoc_strdup(parch);
> +		if (NULL != suffix)
> +			*suffix = '\0';
> +		nof->title = mandoc_strdup(fn);
>
>   		if (verb>  2)
>   			printf("%s: Scheduling\n", buf);
> @@ -1261,6 +1398,9 @@ ofile_free(struct of *of)
>   	while (of) {
>   		nof = of->next;
>   		free(of->fname);
> +		free(of->sec);
> +		free(of->arch);
> +		free(of->title);
>   		free(of);
>   		of = nof;
>   	}
> --
>   To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv
>

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