Re: [Qemu-devel] [PATCH 1/5] char: Add a QemuChrHandlers struct to initialise chardev handlers

[Blue Swirl]

On Thu, Jan 13, 2011 at 6:14 AM, Amit Shah <address@hidden> wrote:
> On (Wed) Jan 12 2011 [19:03:58], Blue Swirl wrote:
>> >>>> +static QemuChrHandlers null_handlers = {
>> >>>> +    /* All handlers are initialised to NULL */
>> >>>> +};
>> >>>> +
>> >>>>  void qemu_chr_add_handlers(CharDriverState *s,
>> >>>> -                           IOCanReadHandler *fd_can_read,
>> >>>> -                           IOReadHandler *fd_read,
>> >>>> -                           IOEventHandler *fd_event,
>> >>>> -                           void *opaque)
>> >>>> -{
>> >>>> -    s->chr_can_read = fd_can_read;
>> >>>> -    s->chr_read = fd_read;
>> >>>> -    s->chr_event = fd_event;
>> >>>> +                           QemuChrHandlers *handlers, void *opaque)
>> >>>> +{
>> >>>
>> >>> Here we could also check if (!s) and return if so. This would simplify
>> >>> the callers a bit.
>> >>
>> >> Simplified in what way?
>> >
>> > I assume for reducing the need to have to check s->chr != NULL everytime
>> > beforehand. It's safer and would save a lot on repetitive code as well.
>>
>> Yes, that's what I meant.
>
> OK, can be added, but I'm wondering if it makes sense (i.e., if an
> assert would be better than return).

No, the idea is that the caller does not have to do the current NULL
checks anymore and can then possibly pass a NULL CharDriverState. This
is of course invalid, but to make things easier for the caller, such
calls are just to be ignored.