zhanghailiang <zhang.zhanghaili...@huawei.com> writes: > On 2014/11/5 15:05, Michael Tokarev wrote: >> 04.11.2014 16:25, Alex Bennée wrote: >>> zhanghailiang <zhang.zhanghaili...@huawei.com> 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