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

[Christophe Fergeau]

On Tue, Dec 18, 2018 at 12:22:31PM -0500, Frediano Ziglio wrote:
> > 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?

It can probably be added just after your first sentence? It's not really
meant as an alternative, but as an expansion of your log.

Christophe

> 
> > > 
> > > 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20181219/43922624/attachment.sig>