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

[Amit Shah]

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).

                Amit