On 4 September 2017 at 14:53, Eric Engestrom <eric.engest...@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.engest...@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.engest...@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/ _______________________________________________ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel