From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=0.0 required=5.0 tests=UNPARSEABLE_RELAY autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 22276 invoked from network); 5 Aug 2021 12:36:04 -0000 Received: from bsd.lv (HELO mandoc.bsd.lv) (66.111.2.12) by inbox.vuxu.org with ESMTPUTF8; 5 Aug 2021 12:36:04 -0000 Received: from fantadrom.bsd.lv (localhost [127.0.0.1]) by mandoc.bsd.lv (OpenSMTPD) with ESMTP id f9de1029 for ; Thu, 5 Aug 2021 07:36:01 -0500 (EST) Received: from scc-mailout-kit-02.scc.kit.edu (scc-mailout-kit-02.scc.kit.edu [129.13.231.82]) by mandoc.bsd.lv (OpenSMTPD) with ESMTP id 76ded751 for ; Thu, 5 Aug 2021 07:36:00 -0500 (EST) Received: from hekate.asta.kit.edu ([141.3.145.153] helo=hekate.usta.de) by scc-mailout-kit-02.scc.kit.edu with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (envelope-from ) id 1mBcbO-0001IG-Il; Thu, 05 Aug 2021 14:35:59 +0200 Received: from donnerwolke.asta.kit.edu ([141.3.145.61] helo=donnerwolke.usta.de) by hekate.usta.de with esmtp (Exim 4.92.2) (envelope-from ) id 1mBcbN-0004hZ-0P; Thu, 05 Aug 2021 14:35:57 +0200 Received: from athene.asta.kit.edu ([141.3.145.60] helo=athene.usta.de) by donnerwolke.usta.de with esmtp (Exim 4.84_2) (envelope-from ) id 1mBcbM-0004GB-Qw; Thu, 05 Aug 2021 14:35:56 +0200 Received: from localhost (athene.usta.de [local]) by athene.usta.de (OpenSMTPD) with ESMTPA id 675330a4; Thu, 5 Aug 2021 14:35:56 +0200 (CEST) Date: Thu, 5 Aug 2021 14:35:56 +0200 From: Ingo Schwarze To: sternenseemann@systemli.org Cc: tech@mandoc.bsd.lv Subject: Re: [PATCH v2 makewhatis] refactor HOMEBREWDIR into READ_ALLOWED_PATH Message-ID: <20210805123556.GA13788@athene.usta.de> References: <8bce7cc9-954e-1c28-ee25-13969f66eb20@systemli.org> <20210330203020.GA94101@athene.usta.de> <060f9222-a388-a9fa-dc34-8f1981f8bf65@systemli.org> <20210331173434.GA57338@athene.usta.de> <3d224309-ff55-6354-8996-2c0e2b38f7a6@systemli.org> <991f74b2-499d-dafe-267b-f2ff5d64e4ee@systemli.org> <9353c0c4-83d9-ae57-0534-81cbcd65632e@systemli.org> X-Mailinglist: mandoc-tech Reply-To: tech@mandoc.bsd.lv MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <9353c0c4-83d9-ae57-0534-81cbcd65632e@systemli.org> User-Agent: Mutt/1.12.2 (2019-09-21) 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