Re: [Qemu-devel] [PATCH v2 1/2] blkdebug: Simplify and improve filename generation

[Max Reitz <mreitz@redhat.com>]

On 2014-11-10 at 20:15, Eric Blake wrote:
> On 11/10/2014 09:59 AM, Max Reitz wrote:
>> Instead of actually recreating the options from scratch, just reuse the
>> options given for creating the BDS, which are the configuration file
>> name and additional options. In case there are no additional options we
>> can thus create a plain filename.
>>
>> This obviously results in a different output for qemu-iotest 099 which
>> exactly tests this filename generation. Fix it up as well.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>   block/blkdebug.c           | 98 +++++++++++++---------------------------------
>>   tests/qemu-iotests/099.out |  4 +-
>>   2 files changed, 29 insertions(+), 73 deletions(-)
>>
>>   
>> +    if (!force_json && bs->file->exact_filename[0]) {
>> +        snprintf(bs->exact_filename, sizeof(bs->exact_filename),
>> +                 "blkdebug:%s:%s", qdict_get_try_str(bs->options, "config"),
> If there is no "config" entry, this can pass NULL as a %s counterpart to
> snprintf, which is undefined behavior (works on glibc, but might crash
> elsewhere).

Well, depends on the definition of "works". "blkdebug:(null):foo" isn't 
that great either.

> Everything else looked okay, but I'm afraid I may have given you reason
> to post a v3 and make a kitten cry :(

I do have a reason to post a v3, but the kitten would have to cry only 
if I had to post a v3 because there were no reviews. This way, 
everything is fine, thank you very much for your reviews (on the other 
series as well). :-)

Max