On Thu, May 04, 2023 at 04:56:46PM +0200, Juan Quintela wrote: > Peter Xu <pet...@redhat.com> wrote: > > On Thu, May 04, 2023 at 01:38:32PM +0200, Juan Quintela wrote: > >> - convince and review code to see that everything is uint64_t. > > > > One general question to patches regarding this - what's the major benefit > > of using uint64_t? > > > > It doubles the possible numbers to hold, but it's already 64bits so I don't > > think it matters a lot. > > We were checking for negatives even when that can't be. > And we are doing this dance of > > int64_t x, y; > uint64_t a, b; > > x = a; > b = y; > > This is always confusing and not always right.
Yeah this is confusing, but if anything can go wrong with this I assume we could have some bigger problem anyway.. > > > The thing is we're removing some code trying to > > detect negative which seems to be still helpful to detect e.g. overflows > > (even though I don't think it'll happen). I just still think it's good to > > know when overflow happens, and not sure what I missed on benefits of using > > unsigned here. > > If you grep through the code, you see that half of the things are > int64_t and the other half is uint64_t. I find it always confusing. Right, I'm personally curious whether we should just use int64_t always unless necessary. :) Another good thing with int64_t is it's also suitable for error report when used in retvals. But no strong opinion here, I don't think that's a huge deal for now. Having such an alignment on types makes sense to me. Thanks, -- Peter Xu
