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

[Masahiro Yamada]

On Sat, Sep 7, 2024 at 10:26 PM Sam James <sam@xxxxxxxxxx> wrote:
>
> Sam James <sam@xxxxxxxxxx> writes:
>
> > Masahiro Yamada <masahiroy@xxxxxxxxxx> writes:
> >
> >> On Sat, Sep 7, 2024 at 12:29 AM Sam James <sam@xxxxxxxxxx> 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
>






Your quotation of the POSIX fread() spec:
(https://pubs.opengroup.org/onlinepubs/000095399/functions/fread.html)

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


I think this is clear enough.


The end-of-file should not be encountered, as we check the file
size in advance.

So, some error might be happening.




-- 
Best Regards
Masahiro Yamada