Re: [Qemu-devel] [PATCH 1/2] block/file-posix: Fix fully preallocated truncate

[Eric Blake]

On 02/28/2018 07:53 AM, Daniel P. Berrangé wrote:

> > > > @@ -1711,8 +1712,8 @@ static int raw_regular_truncate(int fd, int64_t offset, 
> > > > PreallocMode prealloc,
> > > >   
> > > >           buf = g_malloc0(65536);
> > > >   
> > > > -        result = lseek(fd, current_length, SEEK_SET);
> > > > -        if (result < 0) {
> > > > +        seek_result = lseek(fd, current_length, SEEK_SET);
> > > > +        if (seek_result < 0) {
> > >
> > > off_t is an unsigned type, so this comparison to "< 0" is bogus - only the
> > > exact value (off_t)-1 indicates an error. So this needs to be
> > >
> > >     if (seek_result == (off_t)-1) {
> > >        ...
> > >     }

No, off_t is ALWAYS signed, even when it is 32-bit.  The cast helps code 
that is written for 64-bit off_t to still work when compiled with the 
small file model where a 32-bit value is used (but we don't have to 
worry about that, as we always compile for large file mode with 64-bit 
off_t).

> > Hmmm... On my system, it appears to be a long int[1].  And
> > find_allocation() does an off_t < 0 comparison already.  And
> > "man 0p sys_types.h" says "blkcnt_t and off_t shall be signed integer
> > types."
>
> Hmm, that's odd then - lseek man page explicitly said it must be cast,
> which suggested to me it could be unsigned:
>
>     RETURN VALUE
>         Upon successful completion, lseek() returns the resulting offset  loca‐
>         tion  as  measured  in bytes from the beginning of the file.  On error,
>         the value (off_t) -1 is returned and  errno  is  set  to  indicate  the
>         error.
>
> CC'ing Eric for the "official" POSIX answer....

It MAY be that the man page mentions a cast because '-1' is an int but 
'(off_t) -1' can be larger than an int.  But it may also be that you've 
uncovered something worth reporting as a bug to the man page project.


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org