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

[Jim Meyering]

Bruno Haible wrote:
...
>> > 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.

That code tends to confuse me, too.  It is rather tricky.

> If we fix Eric's typo, REPLACE_LGPL will come out as 0 on OpenBSD and NetBSD.

s/REPLACE_LGPL/REPLACE_GETCWD/

Do you know how that happens, given the corrected code?

  case 
$gl_cv_func_getcwd_null,$gl_cv_func_getcwd_posix_signature,$gl_cv_func_getcwd_path_max,$gl_abort_bug
 in
  *yes,yes,yes,no) ;;
  *)
    dnl Full replacement lib/getcwd.c, overrides LGPL replacement.
    REPLACE_GETCWD=1;;
  esac

That means these must be true:
  - they don't have the getcwd abort bug.  agreed: it was glibc/ia64-specific
  - they have a POSIX signature for getcwd: likely
  - getcwd (NULL, 0) works as we expect for names shorter than PATH_MAX:
      easy to believe.

The part that is surprising is that they would have
$gl_cv_func_getcwd_path_max = yes.
That would mean that getcwd (NULL, 0) can succeed when called
from a directory whose name is longer than PATH_MAX.

If that were true, then their getcwd would not be failing with
test_abort_bug() returning 4.  A contradiction.

> 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) ?

Yes, assuming that nothing else goes wrong.

> Or do they have to be prepared to a NULL result?

They should be prepared for NULL, due to the possibility
of ENOMEM, EIO, etc.

>> 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?

Yes.

> 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?

Clarification will help.
However, I think Eric proposed some POSIX wording change that would
allow gnulib's getcwd to be considered compliant.