From: Ingo Schwarze <schwarze@usta.de>
To: "Sören Tempel" <soeren@soeren-tempel.net>
Cc: tech@mandoc.bsd.lv
Subject: Re: Discrepancy in mansearch and fs_lookup behavior
Date: Sat, 4 Sep 2021 15:18:32 +0200 [thread overview]
Message-ID: <20210904131832.GB61031@athene.usta.de> (raw)
In-Reply-To: <2E058N5M5N7IG.2Z5CUW18DPHPS@8pit.net>
Hi Soeren,
Soeren Tempel wrote on Sat, Sep 04, 2021 at 12:12:50PM +0200:
> Ingo Schwarze <schwarze@usta.de> wrote:
>> Hold on a second; if there are mandoc bugs or misfeatures in this area,
>> uprooting major areas of manual page forests in a particular operating
>> system may not be the best option for dealing with that.
> Indeed, I would also prefer not move the Alpine man page. It was just
> unclear to me whether mandoc intended to support lookup for section
> which do not have their own subdirectory in MANDIR
I did not intend to in the past - but merely because i was unaware
of the need to support such a configuration. This is a typical case
of software needing real-world use and testing to mature.
> from reading the fs_lookup code it seemed to me that this was not
> supported, thus my initial idea was to move the pages.
I hope we get this sorted out and you don't need to - hopefully
without breaking the setup in other operating systems either.
Illumos may be particularly fragile, so i hope people will test
the next release over there. And i'd like to see tests on FreeBSD
too, but those will almost certainly happen anyway.
[...]
>> === PAGES ===
>> page name # [fh1t]open
>> page sect # 3 # 3p # 3pm
[...]
> page name # [fh1t]open
> page sect # 3 # 3pm
Looks like your Perl pages contain ".TH open 3" or ".TH open 3pm"
whereas OpenBSD has ".TH open 3p", but i don't expect that to
cause trouble. Actually, not having "3p" in there is good for you.
It lessens the risk of mixups.
[...]
>> Next up, regarding mansearch().
>>
>> man -M Test open -> Perl; random choice because bits, sec, name,
>> and arch all compare equal
>> man -M Test 3 open -> Perl; dto. and both file extensions mismatch
>> man -M Test 3p open -> POSIX; preferred due to exact file extension match
> If I try to duplicate your mansearch() tests I get the following results:
>
> man open -> open(2) Linux man page (/usr/share/man/man2/open.2.gz)
> man 3 open -> open(3pm) Perl man page
> man 3p open -> open(3pm) Perl man page
>
> I get these results with both mandoc-1.14.5 and mandoc from current CVS.
Let's talk about CVS HEAD only, mixing in statements about old releases
can cause nothing but confusion.
[...]
> For `man 3p open` I briefly stepped through the mansearch() code with
> gdb on the mandoc CVS version. What happens here is that mansearch()
> returns the following results:
[... snip correct results ...]
> mandoc then searches for the best match according to priority. In this
> case both resn[0] and resn[1] get a priority of 35. However, since it
> iterates over resn[0] first and the comparison with best_prio is a
> greater than or equal comparison [1], resn[0] is the one that gets
> selected. Maybe this is a problem with the .gz file extension?
Yes, most likely. I shall have a look separately and try to fix that.
[...]
> Regarding fs_search(): I tested the patch from your other reply. This
> patch work entirely fine for me and does what it is supposed to do. I
> applied this patch on top of the CVS version. With mandoc.db removed, I
> get the following results:
>
> man 3 open -> open(3p) POSIX man page
> man 3p open -> open(3p) POSIX man page
> man 3pm open -> open(3pm) Perl man page
>
> So yes, your patch does seem to improve the fs_search() behaviour,
> thanks a lot!
Thank you very much for testing! It is really good to have a real-world
test before commit rather only what i did in my Test/ sandbox.
The patch appended below is now committed.
Hang on a moment for the remaining issue with the .gz extension
probably breaking the prio code, i'll try to figure that out and
send a patch for that, too.
Yours,
Ingo
Log Message:
-----------
In the fallback code to look for manual pages without using mandoc.db(5),
accept files "man<one-digit-section>/<name>.<full-section>"
in addition to the already supported "man<full-section>/name.[01-9]*".
Needed for example on Alpine Linux which puts its Perl manuals
into "man3/<name>.3pm" and the POSIX manuals into "man3/<name>.3p".
While here, allow the glob(3) at the end of fs_lookup() to add multiple
matches to the result set. This improves man -w output and may also
help some cases of plain man(1), allowing main() to prioritize properly
rather than fs_lookup() picking a random match.
Issue reported and patch tested
by Soeren Tempel <soeren at soeren hyphen tempel dot net>.
Modified Files:
--------------
mandoc:
main.c
Revision Data
-------------
Index: main.c
===================================================================
RCS file: /home/cvs/mandoc/mandoc/main.c,v
retrieving revision 1.356
retrieving revision 1.357
diff -Lmain.c -Lmain.c -u -p -r1.356 -r1.357
--- main.c
+++ main.c
@@ -94,9 +94,11 @@ struct outstate {
int mandocdb(int, char *[]);
static void check_xr(struct manpaths *);
-static int fs_lookup(const struct manpaths *,
- size_t ipath, const char *,
- const char *, const char *,
+static void fs_append(char **, size_t, int,
+ size_t, const char *, enum form,
+ struct manpage **, size_t *);
+static int fs_lookup(const struct manpaths *, size_t,
+ const char *, const char *, const char *,
struct manpage **, size_t *);
static int fs_search(const struct mansearch *,
const struct manpaths *, const char *,
@@ -700,6 +702,30 @@ glob_esc(char **dst, const char *src, co
*(*dst)++ = *suffix++;
}
+static void
+fs_append(char **file, size_t filesz, int copy, size_t ipath,
+ const char *sec, enum form form, struct manpage **res, size_t *ressz)
+{
+ struct manpage *page;
+
+ *res = mandoc_reallocarray(*res, *ressz + filesz, sizeof(**res));
+ page = *res + *ressz;
+ *ressz += filesz;
+ for (;;) {
+ page->file = copy ? mandoc_strdup(*file) : *file;
+ page->names = NULL;
+ page->output = NULL;
+ page->bits = NAME_FILE & NAME_MASK;
+ page->ipath = ipath;
+ page->sec = (*sec >= '1' && *sec <= '9') ? *sec - '1' + 1 : 10;
+ page->form = form;
+ if (--filesz == 0)
+ break;
+ file++;
+ page++;
+ }
+}
+
static int
fs_lookup(const struct manpaths *paths, size_t ipath,
const char *sec, const char *arch, const char *name,
@@ -707,16 +733,19 @@ fs_lookup(const struct manpaths *paths,
{
struct stat sb;
glob_t globinfo;
- struct manpage *page;
- char *file, *cp;
+ char *file, *cp, secnum[2];
int globres;
enum form form;
const char *const slman = "/man";
const char *const slash = "/";
const char *const sglob = ".[01-9]*";
+ const char *const dot = ".";
+ const char *const aster = "*";
+ memset(&globinfo, 0, sizeof(globinfo));
form = FORM_SRC;
+
mandoc_asprintf(&file, "%s/man%s/%s.%s",
paths->paths[ipath], sec, name, sec);
if (stat(file, &sb) != -1)
@@ -751,14 +780,34 @@ fs_lookup(const struct manpaths *paths,
mandoc_msg(MANDOCERR_GLOB, 0, 0,
"%s: %s", file, strerror(errno));
free(file);
+ file = NULL;
if (globres == 0)
- file = mandoc_strdup(*globinfo.gl_pathv);
+ goto found;
globfree(&globinfo);
- if (globres == 0) {
- if (stat(file, &sb) != -1)
- goto found;
+
+ if (sec[1] != '\0' && *ressz == 0) {
+ secnum[0] = sec[0];
+ secnum[1] = '\0';
+ cp = file = mandoc_malloc(strlen(paths->paths[ipath]) * 2 +
+ strlen(slman) + strlen(secnum) * 2 + strlen(slash) +
+ strlen(name) * 2 + strlen(dot) +
+ strlen(sec) * 2 + strlen(aster) + 1);
+ glob_esc(&cp, paths->paths[ipath], slman);
+ glob_esc(&cp, secnum, slash);
+ glob_esc(&cp, name, dot);
+ glob_esc(&cp, sec, aster);
+ *cp = '\0';
+ globres = glob(file, 0, NULL, &globinfo);
+ if (globres != 0 && globres != GLOB_NOMATCH)
+ mandoc_msg(MANDOCERR_GLOB, 0, 0,
+ "%s: %s", file, strerror(errno));
free(file);
+ file = NULL;
+ if (globres == 0)
+ goto found;
+ globfree(&globinfo);
}
+
if (res != NULL || ipath + 1 != paths->sz)
return -1;
@@ -770,19 +819,14 @@ fs_lookup(const struct manpaths *paths,
found:
warnx("outdated mandoc.db lacks %s(%s) entry, run %s %s",
name, sec, BINM_MAKEWHATIS, paths->paths[ipath]);
- if (res == NULL) {
+ if (res == NULL)
free(file);
- return 0;
- }
- *res = mandoc_reallocarray(*res, ++*ressz, sizeof(**res));
- page = *res + (*ressz - 1);
- page->file = file;
- page->names = NULL;
- page->output = NULL;
- page->bits = NAME_FILE & NAME_MASK;
- page->ipath = ipath;
- page->sec = (*sec >= '1' && *sec <= '9') ? *sec - '1' + 1 : 10;
- page->form = form;
+ else if (file == NULL)
+ fs_append(globinfo.gl_pathv, globinfo.gl_pathc, 1,
+ ipath, sec, form, res, ressz);
+ else
+ fs_append(&file, 1, 0, ipath, sec, form, res, ressz);
+ globfree(&globinfo);
return 0;
}
--
To unsubscribe send an email to tech+unsubscribe@mandoc.bsd.lv
next prev parent reply other threads:[~2021-09-04 13:18 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-30 19:26 Sören Tempel
2021-09-01 20:35 ` Ingo Schwarze
2021-09-02 19:09 ` Ingo Schwarze
2021-09-04 10:12 ` Sören Tempel
2021-09-04 13:18 ` Ingo Schwarze [this message]
2021-09-04 16:16 ` Ingo Schwarze
2021-09-04 17:51 ` Sören Tempel
2021-09-05 12:47 ` Ingo Schwarze
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210904131832.GB61031@athene.usta.de \
--to=schwarze@usta.de \
--cc=soeren@soeren-tempel.net \
--cc=tech@mandoc.bsd.lv \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).