Re: [PATCH] getcwd-lgpl: fix m4 to match relaxed test for BSD

[Bruno Haible]

Hi Jim,

> > This change has the effect of setting REPLACE_GETCWD to 1 on *all*
> > platforms. A string with only two commas can never math the pattern
> > "*yes,yes,yes,no".
> 
> Obviously a typo.

Yes, sure.

> > Such as on glibc 2.11/Linux or MacOS X or FreeBSD 6.4, OpenBSD 4.9, NetBSD 
> > 5.1:
> >   checking whether getcwd (NULL, 0) allocates memory for result... yes
> >   checking for getcwd with POSIX signature... yes
> >   checking whether getcwd is declared... yes
> >   S["REPLACE_GETCWD"]="1"
> >
> > I'd like to rectify this. The fix is not just to fix the 'case' statement,
> > because on OpenBSD 4.9, NetBSD 5.1 that leads to REPLACE_GETCWD=0 (as 
> > intended)
> > but then the unit test fails with error code 32, i.e. test_abort_bug() 
> > returned 4.
> >
> > On these two platforms,
> >   - The test program from m4/getcwd-abort-bug.m4 fails with exit code 4,
> >   - but the macrology around it considers an error code 4 to be OK
> >     (this is also what the comment says: "This is ok, and expected".
> 
> That is true.  Remember that this is a configure-time test.
> It is testing the system's getcwd.
> Some versions of getcwd do fail in that case, yet we must use them when
> possible, to handle names shorter than PATH_MAX with inaccessible parents.
> In this case, gnulib's getcwd calls the system's getcwd, and handles
> the case in which it fails due to the PATH_MAX limitation.
> 
> >   - Then, if getcwd does not get overridden by gnulib, in 
> > tests/test-getcwd.c
> >     the function test_abort_bug() returns 4 and the test fails.
> 
> That sounds right, because here, we're testing gnulib's replacement getcwd
> function.  With gnulib's getcwd, a return value of 4 from test_abort_bug
> does indicate a failure.
> 
> >   - Finally, the documentation says that the 'getcwd' module fixes the fact
> >     that "This function does not handle long file names (greater than 
> > PATH_MAX)"
> >
> > So, what should we do?
> >   - Change m4/getcwd-abort-bug.m4 to consider an exit code 4 also to be a
> >     failure?
> >   - Or change tests/test-getcwd.c to consider test_abort_bug() = 4 a success
> >     and change the documentation to state that getcwd(NULL,0) may not work
> >     with long file names, even when module 'getcwd' is enabled?
> 
> gnulib's getcwd should never make test_abort_bug() return 4.
> Do you know how/why that happens?

You are confusing me a bit now. Let me state the problem again.

If we fix Eric's typo, REPLACE_LGPL will come out as 0 on OpenBSD and NetBSD.
This means, lib/getcwd.c will not get compiled, and the getcwd() function that
coreutils and tests/test-getcwd.c see will be *exactly* the system function.
But this system function makes test_abort_bug() return 4.

Can packages that use gnulib's getcwd() module - including coreutils -
expect that in normal conditions (no EPERM anywhere)
     mkdir ("subdir1", 0700);
     chdir ("subdir1");
     mkdir ("subdir2", 0700);
     chdir ("subdir2");
     ...
     mkdir ("subdir1000", 0700);
     chdir ("subdir1000");
     dir = getcwd (NULL, 0);
produces a non-NULL dir (longer than PATH_MAX) ?
Or do they have to be prepared to a NULL result?

> Some versions of getcwd do fail in that case, yet we must use them when
> possible, to handle names shorter than PATH_MAX with inaccessible parents.
> In this case, gnulib's getcwd calls the system's getcwd, and handles
> the case in which it fails due to the PATH_MAX limitation.

So what you are saying is that the unit test and the documentation are
correct, and only the getcwd*.m4 files need to be changed so as to activate
the replacement in lib/getcwd.c. This replacement will first call the
system's getcwd() function, so as to handle file names shorter than PATH_MAX
with inaccessible parents.

Is that what you mean?

Then the comment in m4/getcwd-abort-bug.m4

  /* If libc has the bug in question, this invocation of getcwd
     results in a failed assertion.  */
  cwd = getcwd (NULL, 0);
  if (cwd == NULL)
    fail = 4; /* getcwd failed.  This is ok, and expected.  */

needs to be changed to

  /* If libc has the bug in question, this invocation of getcwd
     results in a failed assertion.  */
  cwd = getcwd (NULL, 0);
  if (cwd == NULL)
    fail = 4; /* getcwd failed.  This is sufficient for a POSIX compliant
                 getcwd() function, but as an extension to POSIX, gnulib's
                 getcwd() wants to provide a non-NULL value in this case.  */

Is that right?

Bruno
-- 
In memoriam Ricardo Flores Magón 
<http://en.wikipedia.org/wiki/Ricardo_Flores_Magón>