Re: [PATCH v2 makewhatis] refactor HOMEBREWDIR into READ_ALLOWED_PATH

[Ingo Schwarze <schwarze@usta.de>]

Hi Lukas,

sternenseemann wrote on Tue, Aug 03, 2021 at 06:42:13PM +0200:

> just wanted to bump this thread since it is still important to me.

Thats reasonable, thank you, and sorry for forgetting about it.

> I'd be happy to make any adjustments as well as implement review
> suggestions. So if you have time for a review, that'd be great!

Since i desired several minor tweaks that you could hardly anticipate,
i decided to just do them myself, starting from your ideas and patch.

On 4/10/21 6:15 PM, sternenseemann wrote:

> Hm, this is kinda unnecessary, is it? start will be set again for all
> files which are symlinks, so this value will never get used right?

Yes, i think so.  Besides, the start pointer wasn't advanced in the
past with HOMEBREWDIR set, so i don't quite see why the present
change should suddenly make it necessary.  Even if the pointer were
used for something, stripping a directory that is not the basedir
would seem wrong for any purpose because it would lose information.

Other tweaks:
 * No need for backward compatibility.  I expect the number of people
   to be affected by the change to be exactly one, the mandoc
   maintainer for the Homebrew package manager.  They can trivially
   adjust the variable name in their configure.local when updating
   the port.
 * Let's not forget to initialize READ_ALLOWED_PATH in ./configure.
 * Let's keep lines below 80 characters.
 * Rewrite the comment to not start with one particular example
   now that we can finally describe the feature in a generic way.
 * Using the Nix and Guix package managers at the same time does not
   make much sense, or does it?  So let's show multiple examples
   that actually have a chance of being usable as-is in practice.
 * It is critical for maintenance that the code without READ_ALLOWED_PATH
   remains as undististurbed as possible.  This is a portable-only
   feature that will not be committed to the master repository
   at openbsd.org.
   In particular, the basedir test must not be moved into a new
   function.
 * For better or worse, other functions in this file returning int
   return 1 for success or 0 for failure, so do the same here.
 * The argument of read_allowed() can be const char *.
 * With expressive variable names like READ_ALLOWED_PATH and *candidate,
   there is no real need for a long comment.
 * For a better overview, declare all variables at the top of the
   function, as usual in this codebase.
 * The variable name *cp is customary in this codebase for iterating
   over strings.

Does this version work for you?

Yours,
  Ingo


Index: configure
===================================================================
RCS file: /home/cvs/mandoc/mandoc/configure,v
retrieving revision 1.77
diff -u -p -r1.77 configure
--- configure	20 Jul 2020 16:57:30 -0000	1.77
+++ configure	5 Aug 2021 11:42:59 -0000
@@ -107,7 +107,7 @@ BIN_FROM_SBIN=
 INCLUDEDIR=
 LIBDIR=
 MANDIR=
-HOMEBREWDIR=
+READ_ALLOWED_PATH=
 
 WWWPREFIX="/var/www"
 HTDOCDIR=
@@ -461,7 +461,8 @@ echo "#define MANPATH_DEFAULT \"${MANPAT
 echo "#define OSENUM ${OSENUM}"
 [ -n "${OSNAME}" ] && echo "#define OSNAME \"${OSNAME}\""
 [ -n "${UTF8_LOCALE}" ] && echo "#define UTF8_LOCALE \"${UTF8_LOCALE}\""
-[ -n "${HOMEBREWDIR}" ] && echo "#define HOMEBREWDIR \"${HOMEBREWDIR}\""
+[ -n "${READ_ALLOWED_PATH}" ] \
+	&& echo "#define READ_ALLOWED_PATH \"${READ_ALLOWED_PATH}\""
 [ ${HAVE_ATTRIBUTE} -eq 0 ] && echo "#define __attribute__(x)"
 [ ${HAVE_EFTYPE} -eq 0 ] && echo "#define EFTYPE EINVAL"
 [ ${HAVE_O_DIRECTORY} -eq 0 ] && echo "#define O_DIRECTORY 0"
Index: configure.local.example
===================================================================
RCS file: /home/cvs/mandoc/mandoc/configure.local.example,v
retrieving revision 1.39
diff -u -p -r1.39 configure.local.example
--- configure.local.example	20 Jul 2020 16:57:30 -0000	1.39
+++ configure.local.example	5 Aug 2021 11:43:00 -0000
@@ -209,14 +209,28 @@ INSTALL_LIB="${INSTALL} -m 0444"
 INSTALL_MAN="${INSTALL} -m 0444"
 INSTALL_DATA="${INSTALL} -m 0444"
 
-# When using the "homebrew" package manager on Mac OS X, the actual
-# manuals are located in a so-called "cellar" and only symlinked
-# into the manual trees.  To allow mandoc to follow such symlinks,
-# you have to specify the physical location of the cellar as returned
-# by realpath(3), for example:
+# By default, makewhatis(8) can only read from the paths passed on the
+# command line or configured in man.conf(5).
+# But some package managers on some operating systems store manual pages
+# in separate "cellar" or "store" directories and only symlink them
+# into the manual trees.
+# To support one or more such package managers, give makewhatis(8)
+# read access to the cellars and stores on your system, in the form
+# of a colon-separated path:
 
+# Homebrow package manager on Mac OS X:
 PREFIX="/usr/local"
-HOMEBREWDIR="${PREFIX}/Cellar"
+READ_ALLOWED_PATH="${PREFIX}/Cellar"
+
+# Nix package manager and/or NixOS Linux distibution:
+READ_ALLOWED_PATH="/nix/store"
+
+# GNU Guix package manager and/or GNU Guix Linux distibution:
+READ_ALLOWED_PATH="/gnu/store"
+
+# If multiple package managers are used concurrently:
+PREFIX="/usr/local"
+READ_ALLOWED_PATH="/nix/store:${PREFIX}/Cellar"
 
 # --- user settings for the mandoc(3) library --------------------------
 
Index: mandocdb.c
===================================================================
RCS file: /home/cvs/mandoc/mandoc/mandocdb.c,v
retrieving revision 1.267
diff -u -p -r1.267 mandocdb.c
--- mandocdb.c	3 Apr 2020 11:35:01 -0000	1.267
+++ mandocdb.c	5 Aug 2021 11:43:00 -0000
@@ -165,6 +165,9 @@ static	void	 putkey(const struct mpage *
 static	void	 putkeys(const struct mpage *, char *, size_t, uint64_t);
 static	void	 putmdockey(const struct mpage *,
 			const struct roff_node *, uint64_t, int);
+#ifdef READ_ALLOWED_PATH
+static	int	 read_allowed(const char *);
+#endif
 static	int	 render_string(char **, size_t *);
 static	void	 say(const char *, const char *, ...)
 			__attribute__((__format__ (__printf__, 2, 3)));
@@ -612,8 +615,8 @@ treescan(void)
 				continue;
 			}
 			if (strncmp(buf, basedir, basedir_len) != 0
-#ifdef HOMEBREWDIR
-			    && strncmp(buf, HOMEBREWDIR, strlen(HOMEBREWDIR))
+#ifdef READ_ALLOWED_PATH
+			    && !read_allowed(buf)
 #endif
 			) {
 				if (warnings) say("",
@@ -823,8 +826,8 @@ filescan(const char *infile)
 		start = usefile;
 	else if (strncmp(usefile, basedir, basedir_len) == 0)
 		start = usefile + basedir_len;
-#ifdef HOMEBREWDIR
-	else if (strncmp(usefile, HOMEBREWDIR, strlen(HOMEBREWDIR)) == 0)
+#ifdef READ_ALLOWED_PATH
+	else if (read_allowed(usefile))
 		start = usefile;
 #endif
 	else {
@@ -2380,6 +2383,25 @@ set_basedir(const char *targetdir, int r
 	}
 	return 1;
 }
+
+#ifdef READ_ALLOWED_PATH
+static int
+read_allowed(const char *candidate)
+{
+	const char	*cp;
+	size_t		 len;
+
+	for (cp = READ_ALLOWED_PATH;; cp += len) {
+		while (*cp == ':')
+			cp++;
+		if (*cp == '\0')
+			return 0;
+		len = strcspn(cp, ":");
+		if (strncmp(candidate, cp, len) == 0)
+			return 1;
+	}
+}
+#endif
 
 static void
 say(const char *file, const char *format, ...)
--
 To unsubscribe send an email to tech+unsubscribe@mandoc.bsd.lv