Message ID | 20180420145249.32435-13-peter.maydell@linaro.org |
---|---|
State | New |
Headers | show |
Series | Drop compile time limit on number of serial ports | expand |
On 20/04/2018 16:52, Peter Maydell wrote: > Instead of having a fixed sized global serial_hds[] array, > use a local dynamically reallocated one, so we don't have > a compile time limit on how many serial ports a system has. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Just one question, would it make sense to use a GPtrArray instead? Thanks, Paolo > --- > include/sysemu/sysemu.h | 2 -- > vl.c | 15 +++++++-------- > 2 files changed, 7 insertions(+), 10 deletions(-) > > diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h > index bd5b55c514..989cbc2b7b 100644 > --- a/include/sysemu/sysemu.h > +++ b/include/sysemu/sysemu.h > @@ -161,8 +161,6 @@ void hmp_pcie_aer_inject_error(Monitor *mon, const QDict *qdict); > > #define MAX_SERIAL_PORTS 4 > > -extern Chardev *serial_hds[MAX_SERIAL_PORTS]; > - > /* Return the Chardev for serial port i, or NULL if none */ > Chardev *serial_hd(int i); > > diff --git a/vl.c b/vl.c > index 6daf026da6..a8a98c5a37 100644 > --- a/vl.c > +++ b/vl.c > @@ -154,7 +154,8 @@ QEMUClockType rtc_clock; > int vga_interface_type = VGA_NONE; > static DisplayOptions dpy; > int no_frame; > -Chardev *serial_hds[MAX_SERIAL_PORTS]; > +static int num_serial_hds = 0; > +static Chardev **serial_hds = NULL; > Chardev *parallel_hds[MAX_PARALLEL_PORTS]; > Chardev *virtcon_hds[MAX_VIRTIO_CONSOLES]; > Chardev *sclp_hds[MAX_SCLP_CONSOLES]; > @@ -2496,30 +2497,28 @@ static int foreach_device_config(int type, int (*func)(const char *cmdline)) > > static int serial_parse(const char *devname) > { > - static int index = 0; > + int index = num_serial_hds; > char label[32]; > > if (strcmp(devname, "none") == 0) > return 0; > - if (index == MAX_SERIAL_PORTS) { > - error_report("too many serial ports"); > - exit(1); > - } > snprintf(label, sizeof(label), "serial%d", index); > + serial_hds = g_renew(Chardev *, serial_hds, index + 1); > + > serial_hds[index] = qemu_chr_new(label, devname); > if (!serial_hds[index]) { > error_report("could not connect serial device" > " to character backend '%s'", devname); > return -1; > } > - index++; > + num_serial_hds++; > return 0; > } > > Chardev *serial_hd(int i) > { > assert(i >= 0); > - if (i < ARRAY_SIZE(serial_hds)) { > + if (i < num_serial_hds) { > return serial_hds[i]; > } > return NULL; >
On 04/20/2018 11:52 AM, Peter Maydell wrote: > Instead of having a fixed sized global serial_hds[] array, > use a local dynamically reallocated one, so we don't have > a compile time limit on how many serial ports a system has. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > include/sysemu/sysemu.h | 2 -- > vl.c | 15 +++++++-------- > 2 files changed, 7 insertions(+), 10 deletions(-) > > diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h > index bd5b55c514..989cbc2b7b 100644 > --- a/include/sysemu/sysemu.h > +++ b/include/sysemu/sysemu.h > @@ -161,8 +161,6 @@ void hmp_pcie_aer_inject_error(Monitor *mon, const QDict *qdict); > > #define MAX_SERIAL_PORTS 4 > > -extern Chardev *serial_hds[MAX_SERIAL_PORTS]; > - Finally :))) Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > /* Return the Chardev for serial port i, or NULL if none */ > Chardev *serial_hd(int i); > > diff --git a/vl.c b/vl.c > index 6daf026da6..a8a98c5a37 100644 > --- a/vl.c > +++ b/vl.c > @@ -154,7 +154,8 @@ QEMUClockType rtc_clock; > int vga_interface_type = VGA_NONE; > static DisplayOptions dpy; > int no_frame; > -Chardev *serial_hds[MAX_SERIAL_PORTS]; > +static int num_serial_hds = 0; > +static Chardev **serial_hds = NULL; > Chardev *parallel_hds[MAX_PARALLEL_PORTS]; > Chardev *virtcon_hds[MAX_VIRTIO_CONSOLES]; > Chardev *sclp_hds[MAX_SCLP_CONSOLES]; > @@ -2496,30 +2497,28 @@ static int foreach_device_config(int type, int (*func)(const char *cmdline)) > > static int serial_parse(const char *devname) > { > - static int index = 0; > + int index = num_serial_hds; > char label[32]; > > if (strcmp(devname, "none") == 0) > return 0; > - if (index == MAX_SERIAL_PORTS) { > - error_report("too many serial ports"); > - exit(1); > - } > snprintf(label, sizeof(label), "serial%d", index); > + serial_hds = g_renew(Chardev *, serial_hds, index + 1); > + > serial_hds[index] = qemu_chr_new(label, devname); > if (!serial_hds[index]) { > error_report("could not connect serial device" > " to character backend '%s'", devname); > return -1; > } > - index++; > + num_serial_hds++; > return 0; > } > > Chardev *serial_hd(int i) > { > assert(i >= 0); > - if (i < ARRAY_SIZE(serial_hds)) { > + if (i < num_serial_hds) { > return serial_hds[i]; > } > return NULL; >
On 20 April 2018 at 17:58, Paolo Bonzini <pbonzini@redhat.com> wrote: > On 20/04/2018 16:52, Peter Maydell wrote: >> Instead of having a fixed sized global serial_hds[] array, >> use a local dynamically reallocated one, so we don't have >> a compile time limit on how many serial ports a system has. >> >> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > > Just one question, would it make sense to use a GPtrArray instead? Hmm. Looking at the GPtrArray API there's no API for "tell me the length of this pointer array", so we'd still have to do the manual bookkeeping for that. And we don't need most of the functionality it provides. So it doesn't really seem like it gains us much over g_renew() to me. thanks -- PMM
On 20/04/2018 19:06, Peter Maydell wrote: > On 20 April 2018 at 17:58, Paolo Bonzini <pbonzini@redhat.com> wrote: >> On 20/04/2018 16:52, Peter Maydell wrote: >>> Instead of having a fixed sized global serial_hds[] array, >>> use a local dynamically reallocated one, so we don't have >>> a compile time limit on how many serial ports a system has. >>> >>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> >> >> Just one question, would it make sense to use a GPtrArray instead? > > Hmm. Looking at the GPtrArray API there's no API for > "tell me the length of this pointer array", so we'd still > have to do the manual bookkeeping for that. And we don't > need most of the functionality it provides. So it doesn't > really seem like it gains us much over g_renew() to me. GPtrArray is a public struct, so you can use array->pdata and array->len. There is a disadvantage, which is that you lose type-safety on dereference. Paolo
On 20.04.2018 16:52, Peter Maydell wrote: > Instead of having a fixed sized global serial_hds[] array, > use a local dynamically reallocated one, so we don't have > a compile time limit on how many serial ports a system has. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > include/sysemu/sysemu.h | 2 -- > vl.c | 15 +++++++-------- > 2 files changed, 7 insertions(+), 10 deletions(-) > > diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h > index bd5b55c514..989cbc2b7b 100644 > --- a/include/sysemu/sysemu.h > +++ b/include/sysemu/sysemu.h > @@ -161,8 +161,6 @@ void hmp_pcie_aer_inject_error(Monitor *mon, const QDict *qdict); > > #define MAX_SERIAL_PORTS 4 > > -extern Chardev *serial_hds[MAX_SERIAL_PORTS]; > - > /* Return the Chardev for serial port i, or NULL if none */ > Chardev *serial_hd(int i); > > diff --git a/vl.c b/vl.c > index 6daf026da6..a8a98c5a37 100644 > --- a/vl.c > +++ b/vl.c > @@ -154,7 +154,8 @@ QEMUClockType rtc_clock; > int vga_interface_type = VGA_NONE; > static DisplayOptions dpy; > int no_frame; > -Chardev *serial_hds[MAX_SERIAL_PORTS]; > +static int num_serial_hds = 0; > +static Chardev **serial_hds = NULL; > Chardev *parallel_hds[MAX_PARALLEL_PORTS]; > Chardev *virtcon_hds[MAX_VIRTIO_CONSOLES]; > Chardev *sclp_hds[MAX_SCLP_CONSOLES]; > @@ -2496,30 +2497,28 @@ static int foreach_device_config(int type, int (*func)(const char *cmdline)) > > static int serial_parse(const char *devname) > { > - static int index = 0; > + int index = num_serial_hds; > char label[32]; > > if (strcmp(devname, "none") == 0) > return 0; > - if (index == MAX_SERIAL_PORTS) { > - error_report("too many serial ports"); > - exit(1); > - } > snprintf(label, sizeof(label), "serial%d", index); > + serial_hds = g_renew(Chardev *, serial_hds, index + 1); > + > serial_hds[index] = qemu_chr_new(label, devname); > if (!serial_hds[index]) { In case you respin: I think you could do this without the index variable and just use num_serial_hds directly instead (just a matter of taste, though). Reviewed-by: Thomas Huth <thuth@redhat.com>
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h index bd5b55c514..989cbc2b7b 100644 --- a/include/sysemu/sysemu.h +++ b/include/sysemu/sysemu.h @@ -161,8 +161,6 @@ void hmp_pcie_aer_inject_error(Monitor *mon, const QDict *qdict); #define MAX_SERIAL_PORTS 4 -extern Chardev *serial_hds[MAX_SERIAL_PORTS]; - /* Return the Chardev for serial port i, or NULL if none */ Chardev *serial_hd(int i); diff --git a/vl.c b/vl.c index 6daf026da6..a8a98c5a37 100644 --- a/vl.c +++ b/vl.c @@ -154,7 +154,8 @@ QEMUClockType rtc_clock; int vga_interface_type = VGA_NONE; static DisplayOptions dpy; int no_frame; -Chardev *serial_hds[MAX_SERIAL_PORTS]; +static int num_serial_hds = 0; +static Chardev **serial_hds = NULL; Chardev *parallel_hds[MAX_PARALLEL_PORTS]; Chardev *virtcon_hds[MAX_VIRTIO_CONSOLES]; Chardev *sclp_hds[MAX_SCLP_CONSOLES]; @@ -2496,30 +2497,28 @@ static int foreach_device_config(int type, int (*func)(const char *cmdline)) static int serial_parse(const char *devname) { - static int index = 0; + int index = num_serial_hds; char label[32]; if (strcmp(devname, "none") == 0) return 0; - if (index == MAX_SERIAL_PORTS) { - error_report("too many serial ports"); - exit(1); - } snprintf(label, sizeof(label), "serial%d", index); + serial_hds = g_renew(Chardev *, serial_hds, index + 1); + serial_hds[index] = qemu_chr_new(label, devname); if (!serial_hds[index]) { error_report("could not connect serial device" " to character backend '%s'", devname); return -1; } - index++; + num_serial_hds++; return 0; } Chardev *serial_hd(int i) { assert(i >= 0); - if (i < ARRAY_SIZE(serial_hds)) { + if (i < num_serial_hds) { return serial_hds[i]; } return NULL;
Instead of having a fixed sized global serial_hds[] array, use a local dynamically reallocated one, so we don't have a compile time limit on how many serial ports a system has. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/sysemu/sysemu.h | 2 -- vl.c | 15 +++++++-------- 2 files changed, 7 insertions(+), 10 deletions(-)