diff mbox series

[12/13] vl.c: Remove compile time limit on number of serial ports

Message ID 20180420145249.32435-13-peter.maydell@linaro.org
State Accepted
Commit 6af2692e86f9fdfb3d3d291c2708e81c3125d8e5
Headers show
Series Drop compile time limit on number of serial ports | expand

Commit Message

Peter Maydell April 20, 2018, 2:52 p.m. UTC 
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(-)

-- 
2.17.0

Comments

Paolo Bonzini April 20, 2018, 4:58 p.m. UTC | #1 
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;

>
Philippe Mathieu-Daudé April 20, 2018, 5:01 p.m. UTC | #2 
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;

>
Peter Maydell April 20, 2018, 5:06 p.m. UTC | #3 
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
Paolo Bonzini April 20, 2018, 5:55 p.m. UTC | #4 
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
Thomas Huth April 25, 2018, 3:16 p.m. UTC | #5 
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 mbox series

Patch

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;