Paul Eggert <[email protected]> writes:
> On 2026-06-05 21:15, Collin Funk wrote:
>> Given that the issue exists on all non-EOL macOS versions, I think I am
>> okay with just using "#ifdef __APPLE__". I am leaning towards applying
>> the attached patch, but will sleep on it and give others a chance to
>> review.
>
> That patch doesn't feel right. It opens the file and then fstats it
> and closes it and then opens it again and then checks that the dev+ino
> is the same both ways. Why not just open it once and be done with it?
> There's no need to use fstatat if you gotta open the file anyway.
I guess it might be be possible to move the open earlier, but it would
be a bit of work. Currently, copy_internal stat's (well, fstatat, in
this case) the source file to determine how to operate on it based on
it's type. To make this safe, in copy_reg we check that the dev & inode
don't change inbetween stating and opening the file. Here is the basic
structure of that with error handling removed for brevity:
source_desc = open (src_name, ...);
[...]
fstat (source_desc, &src_open_sb)
[...]
/* SRC_SB is retrieved from fstatat earlier. */
if (! psame_inode (src_sb, &src_open_sb))
{
/* Error. */
}
The issue is that we get a bogus dev/ino from path based stat operations
when invoking 'install' with standard input as the source, for example.
Therefore, my fix used a call to 'open' and used fstat instead of
fstatat. I agree that it is a bit hacky, but the macOS bug causes an
unfortunate issue for 'install' that would be nice to fix. I guess it
has already been broken for 2 years, so no point in rushing a fix. I
will see if I can think of something better.
Collin
