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
