Re: [PATCH 1/6] canonicalize-lgpl: fix EOVERFLOW bug

[Adhemerval Zanella]

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.zanella@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.zanella@linaro.org/