26.05.2014 10:25, Markus Armbruster wrote: > Michael Tokarev <[email protected]> writes: > >> The currrent code in libcacard/vcard_emul_nss.c:vcard_emul_options() >> has a weird bug in variable usage around expanding opts->vreader >> array. >> >> There's a helper variable, vreaderOpt, which is first needlessly >> initialized to NULL, next, conditionally, only we have to expand >> opts->vreader, receives array expansion from g_renew() (initially >> realloc), and next, even if we don't actually perform expansion, > > I don't get the "(initially realloc)" part. The sentence makes sense to > me just fine without it, though.
I was in context of your patch which changes realloc() to g_renew(). And I failed to mention that this my patch is on top of yuors, too - think of this comment as such a mention ;) >> the value of this variable is assigned to the actual array, >> opts->vreader, which was supposed to be expanded. >> >> So, since we expand the array by READER_STEP increments, only >> once in READER_STEP (=4) the code will work, in other 3/4 times >> it will fail badly. >> >> Fix this by not using this temp variable when expanding the >> array, and by dropping the useless =NULL initializer too - >> if it wasn't in place initially, compiler warned us about > > "would have warned us"? Oh yeah. I tried to remember the right English construct, but failed. This tense always escpapes my mind for some reason :) > Much more straightforward now. Thanks! > > Reviewed-by: Markus Armbruster <[email protected]> Thank you. I'll fix the comment. And I'm now ready to push whole -trivial. /mjt
