[libvirt] [PATCH] absolutePathFromBaseFile: don't leak when first arg contains no "/"

Jim Meyering jim at meyering.net
Thu Feb 4 17:27:16 UTC 2010


Daniel Veillard wrote:
>>  absolutePathFromBaseFile(const char *base_file, const char *path)
>>  {
>> -    size_t base_size, path_size;
>> -    char *res, *p;
>> +    char *res;
>> +    size_t d_len = dir_len (base_file);
>>
>> -    if (*path == '/')
>> +    /* If path is already absolute, or if dirname(base_file) is ".",
>> +       just return a copy of path.  */
>> +    if (*path == '/' || d_len == 0)
>>          return strdup(path);
>>
>> -    base_size = strlen(base_file) + 1;
>> -    path_size = strlen(path) + 1;
>> -    if (VIR_ALLOC_N(res, base_size - 1 + path_size) < 0)
>> +    if (VIR_ALLOC_N(res, d_len + 1 + strlen(path) + 1) < 0)
>>          return NULL;
>> -    memcpy(res, base_file, base_size);
>> -    p = strrchr(res, '/');
>> -    if (p != NULL)
>> -        p++;
>> -    else
>> -        p = res;
>> -    memcpy(p, path, path_size);
>> -    if (VIR_REALLOC_N(res, (p + path_size) - res) < 0) {
>> -        /* Ignore failure */
>> -    }
>> +
>> +    stpcpy(stpcpy(stpncpy(res, base_file, d_len), "/"), path);
>>      return res;
>
>   Okay, but while we're cleaning it up, I would suggest to do a
> virReportOOMError(NULL) in case of allocation failure and remove
> the one from virStorageFileGetMetadataFromFD() since it's used only
> once. I think the conn arg is not useful anymore and it's better
> to report the error when it occurs than at the caller level.
>   BTW I didn't know about stp(n)cpy ...

Since the "return strdup(..." can also fail due to an OOM error,
isn't it better to handle all NULL returns in the caller?




More information about the libvir-list mailing list