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

[walter harms]

Am 04.09.2017 13:00, schrieb Emil Velikov:
> 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.
> 
> 

I do not think this is a problem, what the patch catches is the case

	string=ProcessError();
	....
	free(string);

fatal is when the function returns a static string as seen in [PATCH libICE 2/3]
then you are lost. Perhaps the whole think can be converted to a preallocated
buffer, but that would be quit invasive and out of proportion for the problem.

Acked-by: wharms at bfs.de


re,
 wh

>> 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.
> Indentation also seems off, although it could be my MUA.
> 
> With the above nitpicks, the series is
> Reviewed-by: Emil Velikov <emil.velikov at collabora.com>
> 
> -Emil
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: https://lists.x.org/mailman/listinfo/xorg-devel