[Spice-devel] [PATCH spice-common 1/2] Use a single copy of subject string

[Frediano Ziglio]

> On Thu, Dec 13, 2018 at 02:20:24PM +0000, Frediano Ziglio wrote:
> > A full copy can keep both the key and the value instead of
> > allocating twice the memory.
> > Also this remove a warning produced by Coverity that is assuming
> > that allocating strlen(string_variable) is wrong.
> 
> We are parsing key1=val1,key2=val2,... and in doing that, we currently
> store 'key1' in key, and 'val1' in val, and then 'key2', 'val2', and so
> on.
> After your patch, we store 'key1\0val1\0' in key, which fits and saves
> some memory.
> 
> Acked-by: Christophe Fergeau <cfergeau at redhat.com>
> 

Thanks for the review. I assume that these comments are an alternative
comments but there's nothing about Coverity, how should I combine the
2 set of comments together?

> > 
> > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > ---
> >  common/ssl_verify.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/common/ssl_verify.c b/common/ssl_verify.c
> > index 0ac00a6..74f95bb 100644
> > --- a/common/ssl_verify.c
> > +++ b/common/ssl_verify.c
> > @@ -282,7 +282,7 @@ static X509_NAME* subject_to_x509_name(const char
> > *subject, int *nentries)
> >  {
> >      X509_NAME* in_subject;
> >      const char *p;
> > -    char *key, *val, *k, *v = NULL;
> > +    char *key, *val = NULL, *k, *v = NULL;
> >      enum {
> >          KEY,
> >          VALUE
> > @@ -291,11 +291,10 @@ static X509_NAME* subject_to_x509_name(const char
> > *subject, int *nentries)
> >      spice_return_val_if_fail(subject != NULL, NULL);
> >      spice_return_val_if_fail(nentries != NULL, NULL);
> >  
> > -    key = (char*)alloca(strlen(subject));
> > -    val = (char*)alloca(strlen(subject));
> > +    key = (char*)alloca(strlen(subject)+1);
> >      in_subject = X509_NAME_new();
> >  
> > -    if (!in_subject || !key || !val) {
> > +    if (!in_subject || !key) {
> >          spice_debug("failed to allocate");
> >          return NULL;
> >      }
> > @@ -328,6 +327,7 @@ static X509_NAME* subject_to_x509_name(const char
> > *subject, int *nentries)
> >              } else if (*p == '=' && !escape) {
> >                  state = VALUE;
> >                  *k = 0;
> > +                val = k + 1;
> >                  v = val;
> >              } else
> >                  *k++ = *p;

Frediano