On 18/12/2020 09:30, Adhemerval Zanella wrote:
>
>
> On 17/12/2020 07:55, Paul Eggert wrote:
>> On 12/11/20 5:03 AM, Adhemerval Zanella wrote:
>>
>>> I have sent a similar fix to reviews for this very issue for glibc
>>> (which is based on the canonicalize-lgpl) along with other fixes that
>>> you might take a look at [1].
>>
>> Thanks, I looked at that when composing the patches I just now installed
>> into Gnulib, which also attempt to make future glibc merges easier. I think
>> these patches address the issues mentioned in [1] (though sometimes in
>> different ways), except they don't do what this patch does:
>>
>> https://patchwork.sourceware.org/project/glibc/patch/20201027143531.2448132-4-adhemerval.zane...@linaro.org/
>>
>> This patch seems incomplete, since it doesn't address the issue that
>> canonicalize_file_name ("/a/b/.") incorrectly returns "/a/b" even when /a/b
>> is a regular file.
>
> I have suggested using scratch_buffer on some previous iterations a while
> back [1], and although it was incomplete it would be good to had some
> feedback back then to work towards a better solution. I dropped the
> scratch_buffer on the next version [2] because it resulted in a simplified
> code with static stack usage.
>
> Anyway, sometimes I feel glibc is a disjointed project that do not usually
> work together with gnulib, even though it shared a lot of code. I would
> expect that instead of gnulib to just drop a large patch, we would work
> together toward a better solution that suits both projects. Yes, I know
> glibc development side might be slow at times; and maybe it would be better
> if I had sent the patch on gnulib first. But this kind of development where
> glibc side is somewhat ignored makes me wonder if sharing code with gnulib
> is really beneficial.
>
>>
>> Come to think of it, the current Gnulib is suboptimal in that it uses stat
>> ("/a/b/foo/", ...) to test for the existence of a directory /a/b/foo. It
>> could use faccessat (AT_FDCWD, "/a/b/foo/", F_OK, AT_EACCESS), which is
>> often cheaper as it needn't fill in the stat structure.
>>
>> I'll try to make time to look into these two issues, which are somewhat
>> related in the implementations of canonicalize-lgpl and canonicalize.
>
> This is exactly what I am trying to avoid on my latest patch on the
> glibc series. It might be not on par of gnulib code quality standard,
> but it might give you some ideas on how to remove the stat call.
>
> I will drop my patchset and sync with gnulib code. It should at least
> fix BZ #26592 and BZ #26241 and I will work to add the faulty testcase
> you mentioned. The BZ #24970 is mostly a optimization, so we can postpone
> after 2.33 release.
>
> [1] https://sourceware.org/pipermail/libc-alpha/2020-August/117068.html
> [2] https://sourceware.org/pipermail/libc-alpha/2020-September/117522.html
> [3]
> https://patchwork.sourceware.org/project/glibc/patch/20201027143531.2448132-4-adhemerval.zane...@linaro.org/
>
The same tests I pointed out on BZ#24970 comment #2 still fails with gnulib
version 0aa8ef424. I am not sure if it would be better to adapt my original
patchset to use scratch_buffer (which seems originally a better idea) or if
we can work towards fixing on gnulib.
[1]
https://patchwork.sourceware.org/project/glibc/patch/20201027143531.2448132-1-adhemerval.zane...@linaro.org/
