| Message ID | 20180323152948.27048-1-peter.maydell@linaro.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [for-2.12] chardev/char-fe: Allow NULL chardev in qemu_chr_fe_init() | expand |
On 23.03.2018 16:29, Peter Maydell wrote: > All the functions in char-fe.c handle the CharBackend > having a NULL Chardev pointer, which means that the > backend exists but is not connected to anything. The > exception is qemu_chr_fe_init(), which will crash if > passed a NULL Chardev pointer argument. This can happen > for various boards if they're started with 'nodefaults': > arm-softmmu/qemu-system-arm -S -nodefaults -M cubieboard > riscv32-softmmu/qemu-system-riscv32 -nodefaults -M sifive_e > > Make qemu_chr_fe_init() accept a NULL chardev. This allows > UART models to handle NULL chardev properties without > generally needing to special case them or to manually > create a NullChardev. > > Reported-by: Thomas Huth <thuth@redhat.com> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > This is my proposal for fixing the crashes with -nodefaults. > I think we should also change hw/char/serial.c to accept > a NULL Chardev rather than treating it as an error > ( "Can't create serial device, empty char device"), and > similarly for any other devices that currently error out > on NULL chardevs. I'd rather postpone that part til 2.13, though. Sounds like a good plan. Reviewed-by: Thomas Huth <thuth@redhat.com>
On 23/03/2018 16:29, Peter Maydell wrote: > All the functions in char-fe.c handle the CharBackend > having a NULL Chardev pointer, which means that the > backend exists but is not connected to anything. The > exception is qemu_chr_fe_init(), which will crash if > passed a NULL Chardev pointer argument. This can happen > for various boards if they're started with 'nodefaults': > arm-softmmu/qemu-system-arm -S -nodefaults -M cubieboard > riscv32-softmmu/qemu-system-riscv32 -nodefaults -M sifive_e > > Make qemu_chr_fe_init() accept a NULL chardev. This allows > UART models to handle NULL chardev properties without > generally needing to special case them or to manually > create a NullChardev. > > Reported-by: Thomas Huth <thuth@redhat.com> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > This is my proposal for fixing the crashes with -nodefaults. > I think we should also change hw/char/serial.c to accept > a NULL Chardev rather than treating it as an error > ( "Can't create serial device, empty char device"), and > similarly for any other devices that currently error out > on NULL chardevs. I'd rather postpone that part til 2.13, though. > > > chardev/char-fe.c | 22 ++++++++++++---------- > 1 file changed, 12 insertions(+), 10 deletions(-) > > diff --git a/chardev/char-fe.c b/chardev/char-fe.c > index 392db78b13..b1f228e8b5 100644 > --- a/chardev/char-fe.c > +++ b/chardev/char-fe.c > @@ -198,19 +198,21 @@ bool qemu_chr_fe_init(CharBackend *b, Chardev *s, Error **errp) > { > int tag = 0; > > - if (CHARDEV_IS_MUX(s)) { > - MuxChardev *d = MUX_CHARDEV(s); > + if (s) { > + if (CHARDEV_IS_MUX(s)) { > + MuxChardev *d = MUX_CHARDEV(s); > > - if (d->mux_cnt >= MAX_MUX) { > + if (d->mux_cnt >= MAX_MUX) { > + goto unavailable; > + } > + > + d->backends[d->mux_cnt] = b; > + tag = d->mux_cnt++; > + } else if (s->be) { > goto unavailable; > + } else { > + s->be = b; > } > - > - d->backends[d->mux_cnt] = b; > - tag = d->mux_cnt++; > - } else if (s->be) { > - goto unavailable; > - } else { > - s->be = b; > } > > b->fe_open = false; > Queued, thanks. Paolo
diff --git a/chardev/char-fe.c b/chardev/char-fe.c index 392db78b13..b1f228e8b5 100644 --- a/chardev/char-fe.c +++ b/chardev/char-fe.c @@ -198,19 +198,21 @@ bool qemu_chr_fe_init(CharBackend *b, Chardev *s, Error **errp) { int tag = 0; - if (CHARDEV_IS_MUX(s)) { - MuxChardev *d = MUX_CHARDEV(s); + if (s) { + if (CHARDEV_IS_MUX(s)) { + MuxChardev *d = MUX_CHARDEV(s); - if (d->mux_cnt >= MAX_MUX) { + if (d->mux_cnt >= MAX_MUX) { + goto unavailable; + } + + d->backends[d->mux_cnt] = b; + tag = d->mux_cnt++; + } else if (s->be) { goto unavailable; + } else { + s->be = b; } - - d->backends[d->mux_cnt] = b; - tag = d->mux_cnt++; - } else if (s->be) { - goto unavailable; - } else { - s->be = b; } b->fe_open = false;
All the functions in char-fe.c handle the CharBackend having a NULL Chardev pointer, which means that the backend exists but is not connected to anything. The exception is qemu_chr_fe_init(), which will crash if passed a NULL Chardev pointer argument. This can happen for various boards if they're started with 'nodefaults': arm-softmmu/qemu-system-arm -S -nodefaults -M cubieboard riscv32-softmmu/qemu-system-riscv32 -nodefaults -M sifive_e Make qemu_chr_fe_init() accept a NULL chardev. This allows UART models to handle NULL chardev properties without generally needing to special case them or to manually create a NullChardev. Reported-by: Thomas Huth <thuth@redhat.com> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- This is my proposal for fixing the crashes with -nodefaults. I think we should also change hw/char/serial.c to accept a NULL Chardev rather than treating it as an error ( "Can't create serial device, empty char device"), and similarly for any other devices that currently error out on NULL chardevs. I'd rather postpone that part til 2.13, though. chardev/char-fe.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) -- 2.16.2