Re: [Qemu-devel] [PATCH 2/2] xen: Don't peek behind the BlockDriverState abstraction

[Peter Maydell <peter.maydell@linaro.org>]

On 6 June 2012 13:55, Markus Armbruster <armbru@redhat.com> wrote:
> Peter Maydell <peter.maydell@linaro.org> writes:
>
>> On 5 June 2012 13:51, Markus Armbruster <armbru@redhat.com> wrote:
>>> @@ -554,6 +553,7 @@ static int blk_init(struct XenDevice *xendev)
>>>  {
>>>     struct XenBlkDev *blkdev = container_of(xendev, struct XenBlkDev, xendev);
>>>     int index, qflags, info = 0;
>>> +    char fmt_name[128];
>>
>> Fixed length array with a hardcoded magic number size ?
>> If the block layer guarantees that format names are going to be
>> less than 128 bytes it ought to provide a suitable #define for
>> people to set array sizes to...
>
> Maybe it should, but it doesn't.  Does it really matter in this
> particular case?  If somebody insists on giving his driver a name longer
> than 127 characters, we'll silently log it truncated, that's all.

I think it matters in the general case, yours is just the first
usage of this API which has caught my attention. We should fix
the API before adding more uses of it (at the moment it seems to
be only used in two places).

Alternatively, we could have the function return a const char* rather
than taking a buffer to be filled in.

-- PMM