On Wed, Jan 12, 2011 at 6:01 PM, Michael Roth <mdr...@linux.vnet.ibm.com> wrote: > On 01/12/2011 12:07 AM, Amit Shah wrote: >> >> On (Tue) Jan 11 2011 [17:13:15], Blue Swirl wrote: >>>> >>>> +static QemuChrHandlers gdb_handlers = { >>>> + .fd_can_read = gdb_chr_can_receive, >>>> + .fd_read = gdb_chr_receive, >>>> + .fd_event = gdb_chr_event, >>>> +}; >>> >>> These structures should be const. >> >> Hm, I had that but looks like it got lost in some rebase. Added again. >> >>>> @@ -190,15 +190,19 @@ void qemu_chr_send_event(CharDriverState *s, int >>>> event) >>>> s->chr_send_event(s, event); >>>> } >>>> >>>> +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.