On Wed, Jul 14, 2021 at 01:32:21PM -0400, Taylor Blau wrote: > > So if we're already paying for that extra space (which, on some > > platforms would already be a 64 bit int, and on some so would the > > uint32_t, it's just "at least 32 bits"). > > > > Wouldn't it be more idiomatic to just have find_object_pos() return > > int64_t now, if it's -1 it's an error, otherwise the "pos" is cast to > > uint32_t: > > I'm not sure. It does save the extra argument, which is arguably more > convenient for callers, but the cost for doing so is a cast from a > signed integer type to an unsigned one (and a narrower destination type, > at that). > > That seems easier to get wrong to me than passing a pointer to a pure > "int" and keeping the return type a uint32_t. So, I'm probably more > content to leave it as-is rather than change it. I agree that the separate "found" value makes things more obvious. Casting to a smaller size means explaining why that is OK, whereas it is quite clear that things are correct with two separate variables. And having an int64_t makes one wonder whether a value like 2^35 is possible (it isn't; this is a uint32_t because that is the limit of objects in the pack format). -Peff
