Re: [PATCH v2 02/17] block: use int64_t as bytes type in tracked requests

[Philippe Mathieu-Daudé]

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.