
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?