Re: [Qemu-trivial] [Qemu-devel] [PATCH v3 1/5] qemu-char: fix parameter check in some qemu_chr_parse_* functions

[Alex Bennée]

zhanghailiang <address@hidden> writes:

> On 2014/11/5 15:05, Michael Tokarev wrote:
>> 04.11.2014 16:25, Alex Bennée wrote:
>>> zhanghailiang <address@hidden> writes:
<snip>
>> a good reason for having assert here.
>>
>
> Agreed, assert here is still not unnecessary,
> filename will never to be NULL in these two cases.
>
>>> At a minimum I'd replace it with a g_assert(filename) to make the
>>> calling contract clear.
>>
>> This is an internal set of APIs for a chr device, each kind is
>> having a pair of functions which are called in order (first parse,
>> next open), -- _that_ is the contract.

assert isn't really about informing the user, it just makes an explicit
statement that this API will always have a filled in filename and if it
ever gets a NULL that's a programming bug in using the API, internal or
otherwise.

>> []
>>> All this boilerplate checking makes me think that either the qemu_opt
>>> machinery should be ensuring we get a valid option string?
>>
>> Might be a good idea, yes, but that'd be a huge change, since that
>> should be done in a lot of places, and in many cases we can't
>> express our rules easily (eg, only one of two parameters should
>> be present).  I think at this stage adding simple checks to
>> _parse functions is the way to go, and it is easy to read too.

Yes, I wasn't intending to suggest expanding this patch set to encompase
the larger task ;-)

-- 
Alex Bennée