Stefan Hajnoczi <stefa...@gmail.com> writes:

> On Tue, Sep 09, 2014 at 09:45:06AM +0200, mreza...@redhat.com wrote:
>> From: Miroslav Rezanina <mreza...@redhat.com>
>> 
>> It was possible to call strcmp with NULL argument, that can cause
>> segmentation fault. Properly checking parameters to prevent this
>> situation.
>> 
>> Signed-off-by: Miroslav Rezanina <mreza...@redhat.com>
>> ---
>>  util/uri.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>> 
>> diff --git a/util/uri.c b/util/uri.c
>> index e348c17..16c01d0 100644
>> --- a/util/uri.c
>> +++ b/util/uri.c
>> @@ -1985,7 +1985,8 @@ uri_resolve_relative (const char *uri, const char * 
>> base)
>>      val = g_strdup (uri);
>>      goto done;
>>      }
>> -    if (!strcmp(bas->path, ref->path)) {
>> +    if (bas->path != NULL && ref->path != NULL && 
>> +     !strcmp(bas->path, ref->path)) {
>>      val = g_strdup("");
>>      goto done;
>>      }
>
> This patch adds more conditions, what's needed is to audit and clean up this
> function.  There is dead code and things could be simplified instead:
>
>     if (!strcmp(bas->path, ref->path)) {
>         val = g_strdup("");
>         goto done;
>     }
>     if (bas->path == NULL) {
>         val = g_strdup(ref->path);
>         goto done;
>     }
>     if (ref->path == NULL) {
>         ref->path = (char *) "/";
>         remove_path = 1;
>     }
>     <---- bas->path cannot be NULL because we took goto done
>     <---- ref->path cannot be NULL because we assigned "/"
>
>     /*
>      * At this point (at last!) we can compare the two paths
>      *
>      * First we take care of the special case where either of the
>      * two path components may be missing (bug 316224)
>      */
>     if (bas->path == NULL) {  <--- dead code!
>         if (ref->path != NULL) {
>             uptr = ref->path;
>             if (*uptr == '/')
>                 uptr++;
>             /* exception characters from uri_to_string */
>             val = uri_string_escape(uptr, "/;&=+$,");
>         }
>         goto done;
>     }
>     bptr = bas->path;
>     if (ref->path == NULL) {  <--- dead code!
>
> Also, perhaps the strcmp() you touched should really be moved below the NULL
> checks.

If you simplify this function, please get rid of the silly conditionals
around g_strdup().  See commits 80e0090 c14e984 c64f50d for examples.

Reply via email to