[PATCH] man: allow multiple paths in MANPATH

[Marcel Rodrigues]

Thanks for the feedback.

The patch below unifies the code in a separated function. I have considered
using the strchr() approach, but ended up using strtok() because it is
simpler and both approaches required a writable buffer anyway. While it's
possible to refactor the new function to work with read-only buffers (such
as the one directly returned by getenv()), I don't know if it is worth the
trouble -- that would only remove one xstrdup()+free() pair.

Maybe it's also worth mentioning that this patch makes it simple (in
theory) to add a GNU-like option -M to man, which is just another way to
specify a path list. That may be overkill, though.

Signed-off-by: Marcel Rodrigues <marcelgmr at gmail.com>
---
 miscutils/man.c | 67
++++++++++++++++++++++++++-------------------------------
 1 file changed, 31 insertions(+), 36 deletions(-)

diff --git a/miscutils/man.c b/miscutils/man.c
index 5c1fa2c..8959f5e 100644
--- a/miscutils/man.c
+++ b/miscutils/man.c
@@ -147,12 +147,37 @@ static int show_manpage(const char *pager, char
*man_filename, int man, int leve
  return run_pipe(pager, man_filename, man, level);
 }

+static int add_man_paths(char ***man_path_list, int idx, char *man_paths)
+{
+ char *path;
+ if (man_paths == NULL)
+ return idx;
+ path = strtok(man_paths, ":");
+ while (path) {
+ char **path_element;
+ /* Do we already have path? */
+ path_element = *man_path_list;
+ while (*path_element) {
+ if (strcmp(*path_element, path) == 0)
+ goto next;
+ path_element++;
+ }
+ *man_path_list = xrealloc_vector(*man_path_list, 4, idx);
+ (*man_path_list)[idx] = xstrdup(path);
+ idx++;
+ next:
+ path = strtok(NULL, ":");
+ }
+ return idx;
+}
+
 int man_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
 int man_main(int argc UNUSED_PARAM, char **argv)
 {
  parser_t *parser;
  const char *pager = ENABLE_LESS ? "less" : "more";
  char **man_path_list;
+ char *man_paths;
  char *sec_list;
  char *cur_path, *cur_sect;
  int count_mp, cur_mp;
@@ -165,13 +190,12 @@ int man_main(int argc UNUSED_PARAM, char **argv)

  sec_list = xstrdup("0p:1:1p:2:3:3p:4:5:6:7:8:9");
  /* Last valid man_path_list[] is [0x10] */
- count_mp = 0;
  man_path_list = xzalloc(0x11 * sizeof(man_path_list[0]));
- man_path_list[0] = getenv("MANPATH");
- if (!man_path_list[0]) /* default, may be overridden by /etc/man.conf */
+ man_paths = xstrdup(getenv("MANPATH"));
+ count_mp = add_man_paths(&man_path_list, 0, man_paths);
+ free(man_paths);
+ if (!count_mp) /* default, may be overridden by /etc/man.conf */
  man_path_list[0] = (char*)"/usr/man";
- else
- count_mp++;

  /* Parse man.conf[ig] or man_db.conf */
  /* man version 1.6f uses man.config */
@@ -192,37 +216,8 @@ int man_main(int argc UNUSED_PARAM, char **argv)
  } else
  if (strcmp("MANDATORY_MANPATH"+10, token[0]) == 0 /* "MANPATH"? */
  || strcmp("MANDATORY_MANPATH", token[0]) == 0
- ) {
- char *path = token[1];
- while (*path) {
- char *next_path;
- char **path_element;
-
- next_path = strchr(path, ':');
- if (next_path) {
- *next_path = '\0';
- if (next_path++ == path) /* "::"? */
- goto next;
- }
- /* Do we already have path? */
- path_element = man_path_list;
- while (*path_element) {
- if (strcmp(*path_element, path) == 0)
- goto skip;
- path_element++;
- }
- man_path_list = xrealloc_vector(man_path_list, 4, count_mp);
- man_path_list[count_mp] = xstrdup(path);
- count_mp++;
- /* man_path_list is NULL terminated */
- /*man_path_list[count_mp] = NULL; - xrealloc_vector did it */
- skip:
- if (!next_path)
- break;
- next:
- path = next_path;
- }
- }
+ )
+ count_mp = add_man_paths(&man_path_list, count_mp, token[1]);
  if (strcmp("MANSECT", token[0]) == 0) {
  free(sec_list);
  sec_list = xstrdup(token[1]);
-- 
2.1.3


2014-11-26 20:27 GMT-02:00 Denys Vlasenko <vda.linux at googlemail.com>:

> On Wed, Nov 26, 2014 at 7:39 PM, Marcel Rodrigues <marcelgmr at gmail.com>
> wrote:
> > BusyBox's man is reading both /etc/man.config and $MANPATH to search for
> > man-pages, but currently it assumes $MANPATH contains only a single path.
> >
> > The MANPATH environment variable may hold a colon-separated list of
> > directories to be searched by man. The same rationale for PATH
> > applies here: packages installed on different prefixes may coexist on the
> > same system.
>
> You reimplemented the code which already exists a few lines down,
> in config file parser.
>
> Can you refactor the code so that you use a single copy of it for both
> purposes?
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/busybox/attachments/20141126/1922409b/attachment-0001.html>