[PATCH libICE 3/3] Make sure string is never NULL

Eric Engestrom eric.engestrom at imgtec.com
Mon Sep 4 13:53:28 UTC 2017


On Monday, 2017-09-04 12:00:58 +0100, Emil Velikov wrote:
> On 7 July 2017 at 11:23, Eric Engestrom <eric.engestrom at imgtec.com> wrote:
> > `error_message` is passed in to strncpy() without any check, which
> > doesn't handle NULL itself, so let's make it a valid empty string in
> > cases where it was NULL.
> >
> Strictly speaking strdup() can fail, thus we could still end with a NULL.
> In all fairness I'm not sure how much one should bother though.

strdup() failing is further down the list of priorities than explicitly
passing NULL to a function that can't handle NULL ;)

Also note that I didn't experience this crash, I was just bothered by
the compiler warnings whenever I compiled this project :P

> > Signed-off-by: Eric Engestrom <eric.engestrom at imgtec.com>
> > ---
> >  src/process.c | 14 ++++++++++++--
> >  1 file changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/process.c b/src/process.c
> > index 1ee1ceb..30d073f 100644
> > --- a/src/process.c
> > +++ b/src/process.c
> > @@ -704,6 +704,11 @@ ProcessError (
> >                 invokeHandler = 1;
> >             }
> >
> > +                       if (!errorStr)
> > +                       {
> > +                               errorStr = strdup("");
> > +                       }
> > +
> >             errorReply->type = ICE_CONNECTION_ERROR;
> >             errorReply->error_message = errorStr;
> >         }
> > @@ -794,6 +799,11 @@ ProcessError (
> >                 invokeHandler = 1;
> >             }
> >
> > +                       if (!errorStr)
> > +                       {
> > +                               errorStr = strdup("");
> > +                       }
> > +
> Skimming through the file, one could drop the curly brackets.

I'm used to our internal style, which is to always have the brackets,
and this file has a mix of both styles, so I chose to have the explicit
brackets, but I could indeed drop them.
I won't send a v2 just for this though, unless you feel strongly about it?

> Indentation also seems off, although it could be my MUA.

Indentation is all over the place in this file, with random mixes of
tabs and spaces on the surrounding lines, but I used a simple
tab-per-indentation-level on the lines I added.

> 
> With the above nitpicks, the series is
> Reviewed-by: Emil Velikov <emil.velikov at collabora.com>
> 
> -Emil


More information about the xorg-devel mailing list