Re: [Qemu-devel] [PATCH 01/23] block: Split bdrv_new_named() off bdrv_new()

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> On 09/10/2014 05:03 AM, Benoît Canet wrote:
>> The Wednesday 10 Sep 2014 à 10:13:30 (+0200), Markus Armbruster wrote :
>>> Creating an anonymous BDS can't fail.  Make that obvious.
>>>
>>> Signed-off-by: Markus Armbruster <address@hidden>
>>> ---
>
>>>  /* create a new block device (by default it is empty) */
>>> -BlockDriverState *bdrv_new(const char *device_name, Error **errp)
>>> +BlockDriverState *bdrv_new_named(const char *device_name, Error **errp)
>>>  {
>>>      BlockDriverState *bs;
>>> -    int i;
>>> +
>>> +    assert(*device_name);
>> 
>> This assert that device_name is not a null pointer.
>
> No, it assumes that device_name is non-NULL and blindly dereferences it;
> it is asserting that the first byte of the dereferenced pointer is not NUL.
>
>> But here we are pretty sure that the BDS should be named given the
>> name of the function.
>> Should we bake an assert on device_name[0] != '\0' to avoid
>> bdrv_new_named being called
>> with "" as device_name ?
>
> That's already what the code does.  About the only thing it could do
> differently is:
>
> assert(device_name && *device_name);

Converts a crash into an assertion failure.  No difference to me, but if
you guys think it's a worthwhile improvement...