Daniel Veillard wrote:
On Fri, Feb 05, 2010 at 10:59:23AM +0100, Jim Meyering wrote:
Daniel P. Berrange wrote:
Any code which uses a combination of VIR_ALLOC + strcat/strcpy/strpcpy/etc really ought to be re-written to use virAsprintf(). The nested stpcpy is a nice trick, but its not really helping clarity like asprintf would.
Good point. In spite of having to use "%.*s" in the format, using virAsprintf is both more readable and more concise. Note that there is no need to test the return value of virAsprintf here. Here's the incremental patch, followed by the amended one:
Note also that I've done it this way:
virAsprintf(&res, "%.*s/%s", base_file, d_len, path);
but I could also have omitted the "/" from the format, since there's guaranteed to be one at base_file[d_len], but that is not obvious, which makes this much less readable:
virAsprintf(&res, "%.*s%s", base_file, d_len+1, path);
Likewise, I could have avoided one of those two stpcpy calls. ... Subject: [PATCH] absolutePathFromBaseFile: don't leak when first arg contains no "/"
* src/util/storage_file.c: Include "dirname.h". (absolutePathFromBaseFile): Rewrite not to leak, and to require fewer allocations. * bootstrap (modules): Add dirname-lgpl. * .gnulib: Update submodule to the latest.
ACK,
Thanks. Pushed.