From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43580) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wnu10-0007cr-NA for qemu-devel@nongnu.org; Fri, 23 May 2014 14:16:28 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Wnu0u-0002gu-In for qemu-devel@nongnu.org; Fri, 23 May 2014 14:16:22 -0400 Received: from isrv.corpit.ru ([86.62.121.231]:59111) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wnu0u-0002gi-CY for qemu-devel@nongnu.org; Fri, 23 May 2014 14:16:16 -0400 Message-ID: <537F906E.4010101@msgid.tls.msk.ru> Date: Fri, 23 May 2014 22:16:14 +0400 From: Michael Tokarev MIME-Version: 1.0 References: <1400844279-1685-1-git-send-email-armbru@redhat.com> <1400844279-1685-7-git-send-email-armbru@redhat.com> <537F8EF4.9000803@msgid.tls.msk.ru> In-Reply-To: <537F8EF4.9000803@msgid.tls.msk.ru> Content-Type: text/plain; charset=KOI8-R Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 6/7] libcacard/vcard_emul_nss: Assert vreaderOpt isn't null List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster , qemu-devel@nongnu.org Cc: alevy@redhat.com 23.05.2014 22:09, Michael Tokarev wrote: > 23.05.2014 15:24, Markus Armbruster wrote: >> It's not locally obvious, and Coverity can't see it either. >> >> Signed-off-by: Markus Armbruster >> Reviewed-by: Alon Levy >> --- >> libcacard/vcard_emul_nss.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/libcacard/vcard_emul_nss.c b/libcacard/vcard_emul_nss.c >> index 2048917..4f55e44 100644 >> --- a/libcacard/vcard_emul_nss.c >> +++ b/libcacard/vcard_emul_nss.c >> @@ -1181,6 +1181,7 @@ vcard_emul_options(const char *args) >> vreaderOpt = g_renew(VirtualReaderOptions, opts->vreader, >> reader_count); >> } >> + assert(vreaderOpt); >> opts->vreader = vreaderOpt; >> vreaderOpt = &vreaderOpt[opts->vreader_count]; >> vreaderOpt->name = g_strndup(name, name_length); > > Shouldn't the assignment be moved up one line into the if {} > statement instead? Actually it looks like this code is just buggy, it works for just one iteration. Because at this place, vreaderOpts will be non-NULL only if we expanded the array. If we didn't (we do that in READER_STEP increments), we'll be assigning NULL to opts->vreader, because vreaderOpt will _really_ be NULL here. One good example when setting initial values like this (for vreaderOpts) is a Bad Idea (tm). If it weren't needlessly NULL initially, compiler was able to tell us about using uninitialized variable. Thanks, /mjt