[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-trivial] [Qemu-devel] [PATCH] libcacard: fix wrong array expan
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-trivial] [Qemu-devel] [PATCH] libcacard: fix wrong array expansion logic |
Date: |
Mon, 26 May 2014 08:25:24 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) |
Michael Tokarev <address@hidden> 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 <address@hidden>
> ---
> 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 <address@hidden>