Re: [PATCH] fixdep: handle short reads in read_file

[Sam James <sam@gentoo.org>]

Sam James <sam@gentoo.org> writes:

> Masahiro Yamada <masahiroy@kernel.org> writes:
>
>> On Sat, Sep 7, 2024 at 12:29 AM Sam James <sam@gentoo.org> wrote:
>
> Hi Masahiro,
>
>>>
>>> 50% or so of kernel builds within our package manager fail for me with
>>> 'fixdep: read: success' because read(), for some reason - possibly ptrace,
>>> only read a short amount, not the full size.
>>>
>>> Unfortunately, this didn't trigger a -Wunused-result warning because
>>> we _are_ checking the return value, but with a bad comparison (it's completely
>>> fine for read() to not read the whole file in one gulp).
>>>
>>> Fixes: 01b5cbe7012fb1eeffc5c143865569835bcd405e
>>
>>
>> Fixes: 01b5cbe7012f ("fixdep: use malloc() and read() to load dep_file
>> to buffer")
>>
>
> Ah, thanks. I'll fix that and send v2 depending on how we decide to move
> forward wrt below.
>
>>
>> I guess, another approach would be to use fread() instead of read().
>>
>> Does the attached diff fix the issue too?
>>
>>
>
> Unfortunately no. It failed for me in the same way as before :(
>
> The man page mentions:
>> On  success, fread() and fwrite() return the number of items read or
>> written. This number equals the number of bytes transferred only when size is 1.  
>
> so I guess it suffers from the same pitfall. I checked POSIX & ISO C as well
> which says:
>> If a partial element is read, its value is unspecified.
> and
>> The fread() function shall return the number of elements successfully
>> read, which shall be less than nitems only if an error or end-of-file
>> is encountered, or size is zero.
>
> The error reference is kind of mysterious there though.
>
> It kind of looks like fread *should* work. I'll send this mail and then
> think about it a bit later and ask around to see if I'm missing
> something obvious?

OK, others disagree with my reading of fread and think it is ambiguous.

With your patch, I was able to get failures albeit possibly less
frequently. I'm trying my patch again in a loop now.

>
>> [...]
>
> thanks,
> sam