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

[Emil Velikov]

On 4 September 2017 at 14:53, Eric Engestrom <eric.engestrom at imgtec.com> wrote:
> 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
>
Agreed.

>> > 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.
>
Hmm you're right. I wouldn't bother with my nits.

FWIW here's an ancient series of mine, waiting for review [1].
First patch is a little sensitive, but the rest should be trivial.

-Emil
[1] https://patchwork.freedesktop.org/series/6863/