Re: [Qemu-devel] [PATCH] libcacard: Drop superfluous conditionals around g_free()

[Paolo Bonzini]

Il 06/06/2014 18:59, Markus Armbruster ha scritto:
> Paolo Bonzini <address@hidden> writes:
>
> > Il 06/06/2014 18:32, Markus Armbruster ha scritto:
> > > Signed-off-by: Markus Armbruster <address@hidden>
> > > ---
> > >  libcacard/cac.c            | 14 ++++----------
> > >  libcacard/card_7816.c      | 12 +++---------
> > >  libcacard/vcard.c          | 10 +++-------
> > >  libcacard/vcard_emul_nss.c |  4 +---
> > >  libcacard/vreader.c        |  4 +---
> > >  5 files changed, 12 insertions(+), 32 deletions(-)
> > >
> > > diff --git a/libcacard/cac.c b/libcacard/cac.c
> > > index 0a0163d..ae8c378 100644
> > > --- a/libcacard/cac.c
> > > +++ b/libcacard/cac.c
> > > @@ -100,10 +100,8 @@ cac_applet_pki_reset(VCard *card, int channel)
> > >      pki_applet = &(applet_private->u.pki_data);
> > >
> > >      pki_applet->cert_buffer = NULL;
> > > -    if (pki_applet->sign_buffer) {
> > > -        g_free(pki_applet->sign_buffer);
> > > -        pki_applet->sign_buffer = NULL;
> > > -    }
> > > +    g_free(pki_applet->sign_buffer);
> > > +    pki_applet->sign_buffer = NULL;
> > >      pki_applet->cert_buffer_len = 0;
> > >      pki_applet->sign_buffer_len = 0;
> > >      return VCARD_DONE;
> > > @@ -285,12 +283,8 @@ cac_delete_pki_applet_private(VCardAppletPrivate 
> > > *applet_private)
> > >          return;
> > >      }
> > >      pki_applet_data = &(applet_private->u.pki_data);
> > > -    if (pki_applet_data->cert != NULL) {
> > > -        g_free(pki_applet_data->cert);
> > > -    }
> > > -    if (pki_applet_data->sign_buffer != NULL) {
> > > -        g_free(pki_applet_data->sign_buffer);
> > > -    }
> > > +    g_free(pki_applet_data->cert);
> > > +    g_free(pki_applet_data->sign_buffer);
> > >      if (pki_applet_data->key != NULL) {
> > >          vcard_emul_delete_key(pki_applet_data->key);
> > >      }
> > > diff --git a/libcacard/card_7816.c b/libcacard/card_7816.c
> > > index a54f880..814fa16 100644
> > > --- a/libcacard/card_7816.c
> > > +++ b/libcacard/card_7816.c
> > > @@ -172,16 +172,12 @@ vcard_response_delete(VCardResponse *response)
> > >      switch (response->b_type) {
> > >      case VCARD_MALLOC:
> > >          /* everything was malloc'ed */
> > > -        if (response->b_data) {
> > > -            g_free(response->b_data);
> > > -        }
> > > +        g_free(response->b_data);
> > >          g_free(response);
> > >          break;
> > >      case VCARD_MALLOC_DATA:
> > >          /* only the data buffer was malloc'ed */
> > > -        if (response->b_data) {
> > > -            g_free(response->b_data);
> > > -        }
> > > +        g_free(response->b_data);
> > >          break;
> > >      case VCARD_MALLOC_STRUCT:
> > >          /* only the structure was malloc'ed */
> > > @@ -358,9 +354,7 @@ vcard_apdu_delete(VCardAPDU *apdu)
> > >      if (apdu == NULL) {
> > >          return;
> > >      }
> > > -    if (apdu->a_data) {
> > > -        g_free(apdu->a_data);
> > > -    }
> > > +    g_free(apdu->a_data);
> > >      g_free(apdu);
> > >  }
> > >
> > > diff --git a/libcacard/vcard.c b/libcacard/vcard.c
> > > index 6aaf085..bf342aa 100644
> > > --- a/libcacard/vcard.c
> > > +++ b/libcacard/vcard.c
> > > @@ -51,9 +51,7 @@ vcard_buffer_response_delete(VCardBufferResponse 
> > > *buffer_response)
> > >      if (buffer_response == NULL) {
> > >          return;
> > >      }
> > > -    if (buffer_response->buffer) {
> > > -        g_free(buffer_response->buffer);
> > > -    }
> > > +    g_free(buffer_response->buffer);
> > >      g_free(buffer_response);
> > >  }
> > >
> > > @@ -121,10 +119,8 @@ vcard_delete_applet(VCardApplet *applet)
> > >          applet->applet_private_free(applet->applet_private);
> > >          applet->applet_private = NULL;
> >
> > Dead store here...
> >
> > >      }
> > > -    if (applet->aid) {
> > > -        g_free(applet->aid);
> > > -        applet->aid = NULL;
> > > -    }
> > > +    g_free(applet->aid);
> > > +    applet->aid = NULL;
> >
> > ... and here (didn't check if there are more out of context).
> >
> > As usual, no good deed goes unpunished!
>
> Preexisting.  I can try to clean them up in a follow-up patch.  Okay?

Sure.

Paolo