On Sat, Sep 11 2021, Jeff King wrote: > We converted argv_array (which later became strvec) to use size_t in > 819f0e76b1 (argv-array: use size_t for count and alloc, 2020-07-28) in > order to avoid the possibility of integer overflow. But later, commit > d70a9eb611 (strvec: rename struct fields, 2020-07-28) accidentally > converted these back to ints! > > Those two commits were part of the same patch series. I'm pretty sure > what happened is that they were originally written in the opposite order > and then cleaned up and re-ordered during an interactive rebase. And > when resolving the inevitable conflict, I mistakenly took the "rename" > patch completely, accidentally dropping the type change. > > We can correct it now; better late than never. > > Signed-off-by: Jeff King <peff@xxxxxxxx> > --- > This was posted previously in the midst of another thread, but I don't > think was picked up. There was some positive reaction, but one "do we > really need this?" to which I responded in detail: > > https://lore.kernel.org/git/YTIBnT8Ue1HZXs82@xxxxxxxxxxxxxxxxxxxxxxx/ > > I don't really think any of that needs to go into the commit message, > but if that's a hold-up, I can try to summarize it (though I think > referring to the commit which _already_ did this and was accidentally > reverted would be sufficient). Thanks, I have a WIP version of this outstanding starting with this patch that I was planning to submit sometime, but I'm happy to have you pursue it, especially with the ~100 outstanding patches I have in master..seen. It does feel somewhere between iffy and a landmine waiting to be stepped on to only convert the member itself, and not any of the corresponding "int" variables that track it to "size_t". If you do the change I suggested in https://lore.kernel.org/git/87v93i8svd.fsf@xxxxxxxxxxxxxxxxxxx/ you'll find that there's at least one first-order reference to this that now uses "int" that if converted to "size_t" will result in a wrap-around error, we're lucky that one has a test failure. I can tell you what that bug is, but maybe it's better if you find it yourself :) I.e. I found *that* one, but I'm not sure I found them all. I just s/int nr/size_t *nr/ and eyeballed the wall off compiler errors & the code context (note: pointer, obviously broken, but makes the compiler yell). That particular bug will be caught by the compiler as it involves a >= 0 comparison against unsigned, but we may not not have that everywhere...
