On 4/27/20 1:26 PM, Vladimir Sementsov-Ogievskiy wrote:
27.04.2020 13:11, Philippe Mathieu-Daudé wrote:
On 4/27/20 10:23 AM, Vladimir Sementsov-Ogievskiy wrote:
We are generally moving to int64_t for both offset and bytes parameters
on all io paths. Convert tracked requests now.
This doesn't seem a strong justification... If I understand correctly
this patch, it is safer to use positive signed type rather than
unsigned type. OK it might make sense to better catch overflow, but it
should be explained in the function prototypes, else commit message,
else the series cover IMHO.
First time I decided to follow the tendency not to copy the whole
cover-letter from previous series, but just give a link to it :) It's
chosen not for safety..
My reason is the fact that some functions may return int64_t
offset/bytes, and negative values are used to indicate an error. It
seems good to use same type always, making it simple to reuse local
variables for storing return value and as arguments (if it is
appropriate in the context).
I agree we want errors returned, so negative values for that.
I'm not sure it is a good practice to pass unsigned values via signed
type simply to 'reuse' local variables. I'm worried we might hide new
bugs and it might become harder to find others bugs.
Anyway I'll follow Eric and Stefan choice here, as they are more
experienced.
Eric also added (in v1 thread), that off_t is signed too.
So the aim of the series is not signed type, the aim is 64bit. And for
consistency, we should use same type for all io functions. And my
proposal is int64_t, for these two reasons above. May be good to add
them to the first commit message.