On Mon, 15 Jan 2024 at 16:14, Peter Maydell <peter.mayd...@linaro.org> wrote:
>
> (I've cc'd a few people who might have opinions on possible
> command-line compatibility breakage.)
>
> On Wed, 10 Jan 2024 at 14:38, Bohdan Kostiv <bogdan.kos...@gmail.com> wrote:
> >
> > Hello,
> >
> > I have faced an issue in using serial ports when I need to skip a couple of
> > ports in the CLI.
> >
> > For example the ARM machine netduinoplus2 supports up to 7 UARTS.
> > Following case works (the first UART is used to send data in the firmware):
> > qemu-system-arm -machine netduinoplus2 -nographic -serial mon:stdio -kernel
> > path-to-fw/firmware.elf
> > But this one doesn't (the third UART is used to send data in the firmware):
> > qemu-system-arm -machine netduinoplus2 -nographic -serial none -serial none
> > -serial mon:stdio -kernel path-to-fw/firmware.elf
>
> Putting the patch inline for more convenient discussion:
>
> > Subject: [PATCH] Fixed '-serial none' usage breaks following '-serial ...'
> > usage
> >
> > Signed-off-by: Bohdan Kostiv <bohdan.kos...@tii.ae>
> > ---
> > system/vl.c | 5 ++++-
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/system/vl.c b/system/vl.c
> > index 2bcd9efb9a..b8744475cd 100644
> > --- a/system/vl.c
> > +++ b/system/vl.c
> > @@ -1442,8 +1442,11 @@ static int serial_parse(const char *devname)
> > int index = num_serial_hds;
> > char label[32];
> >
> > - if (strcmp(devname, "none") == 0)
> > + if (strcmp(devname, "none") == 0) {
> > + num_serial_hds++;
> > return 0;
> > + }
> > +
> > snprintf(label, sizeof(label), "serial%d", index);
> > serial_hds = g_renew(Chardev *, serial_hds, index + 1);
> >
While I was testing this patch, I discovered that it has a bug:
if you run 'qemu-system-x86_64 -serial none' it now crashes.
This happens because the serial_hd() function assumes that
serial_hds points to enough memory for num_serial_hds
pointers, and now we are increasing num_serial_hds but
skipping the g_renew() that enlarges the array.
I'll send a patch which gets that part right and also has
an expanded commit message which mentions some of the
things we've discussed in this thread.
thanks
-- PMM
