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. > 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"? > this problem at the beginning. > > Signed-off-by: Michael Tokarev <[email protected]> > --- > libcacard/vcard_emul_nss.c | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > > diff --git a/libcacard/vcard_emul_nss.c b/libcacard/vcard_emul_nss.c > index b7db51d..8462aef 100644 > --- a/libcacard/vcard_emul_nss.c > +++ b/libcacard/vcard_emul_nss.c > @@ -1149,7 +1149,7 @@ vcard_emul_options(const char *args) > char type_str[100]; > VCardEmulType type; > int count, i; > - VirtualReaderOptions *vreaderOpt = NULL; > + VirtualReaderOptions *vreaderOpt; > > args = strip(args + 5); > if (*args != '(') { > @@ -1173,11 +1173,10 @@ vcard_emul_options(const char *args) > > if (opts->vreader_count >= reader_count) { > reader_count += READER_STEP; > - vreaderOpt = g_renew(VirtualReaderOptions, opts->vreader, > - reader_count); > + opts->vreader = g_renew(VirtualReaderOptions, opts->vreader, > + reader_count); > } > - opts->vreader = vreaderOpt; > - vreaderOpt = &vreaderOpt[opts->vreader_count]; > + vreaderOpt = &opts->vreader[opts->vreader_count]; > vreaderOpt->name = g_strndup(name, name_length); > vreaderOpt->vname = g_strndup(vname, vname_length); > vreaderOpt->card_type = type; Much more straightforward now. Thanks! Reviewed-by: Markus Armbruster <[email protected]>
