| Message ID | 1354888531-47836-2-git-send-email-jfrei@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
On 07.12.2012, at 14:55, Jens Freimann wrote: > From: Christian Borntraeger <borntraeger@de.ibm.com> > > Since commit 967c0da73a7b0da186baba6632301d83644a570c > vl.c: Avoid segfault when started with no arguments > > the user can specify a kernel without a command line. Lets not > overwrite the default command line with \0 in that case. > > Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> > Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com> > --- > hw/s390-virtio.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c > index ca1bb09..d77871a 100644 > --- a/hw/s390-virtio.c > +++ b/hw/s390-virtio.c > @@ -290,7 +290,7 @@ static void s390_init(QEMUMachineInitArgs *args) > stq_p(rom_ptr(INITRD_PARM_SIZE), initrd_size); > } > > - if (rom_ptr(KERN_PARM_AREA)) { > + if (rom_ptr(KERN_PARM_AREA) && strlen(kernel_cmdline)) { why strlen()? If no -append option was passed, kernel_cmdline should be NULL. If -append "" was passed, the user wants to command line to be overwritten with "\0". Alex
On 11/12/12 11:34, Alexander Graf wrote: > > On 07.12.2012, at 14:55, Jens Freimann wrote: > >> From: Christian Borntraeger <borntraeger@de.ibm.com> >> >> Since commit 967c0da73a7b0da186baba6632301d83644a570c >> vl.c: Avoid segfault when started with no arguments >> >> the user can specify a kernel without a command line. Lets not >> overwrite the default command line with \0 in that case. >> >> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> >> Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com> >> --- >> hw/s390-virtio.c | 2 +- >> 1 files changed, 1 insertions(+), 1 deletions(-) >> >> diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c >> index ca1bb09..d77871a 100644 >> --- a/hw/s390-virtio.c >> +++ b/hw/s390-virtio.c >> @@ -290,7 +290,7 @@ static void s390_init(QEMUMachineInitArgs *args) >> stq_p(rom_ptr(INITRD_PARM_SIZE), initrd_size); >> } >> >> - if (rom_ptr(KERN_PARM_AREA)) { >> + if (rom_ptr(KERN_PARM_AREA) && strlen(kernel_cmdline)) { > > why strlen()? If no -append option was passed, kernel_cmdline should be NULL. If -append "" was passed, the user wants to command line to be overwritten with "\0". Nope. kernel_cmdline is always a valid pointer. vl.c: [..] if (!kernel_cmdline) { kernel_cmdline = ""; } [..]
On 11.12.2012, at 12:15, Christian Borntraeger wrote: > On 11/12/12 11:34, Alexander Graf wrote: >> >> On 07.12.2012, at 14:55, Jens Freimann wrote: >> >>> From: Christian Borntraeger <borntraeger@de.ibm.com> >>> >>> Since commit 967c0da73a7b0da186baba6632301d83644a570c >>> vl.c: Avoid segfault when started with no arguments >>> >>> the user can specify a kernel without a command line. Lets not >>> overwrite the default command line with \0 in that case. >>> >>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> >>> Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com> >>> --- >>> hw/s390-virtio.c | 2 +- >>> 1 files changed, 1 insertions(+), 1 deletions(-) >>> >>> diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c >>> index ca1bb09..d77871a 100644 >>> --- a/hw/s390-virtio.c >>> +++ b/hw/s390-virtio.c >>> @@ -290,7 +290,7 @@ static void s390_init(QEMUMachineInitArgs *args) >>> stq_p(rom_ptr(INITRD_PARM_SIZE), initrd_size); >>> } >>> >>> - if (rom_ptr(KERN_PARM_AREA)) { >>> + if (rom_ptr(KERN_PARM_AREA) && strlen(kernel_cmdline)) { >> >> why strlen()? If no -append option was passed, kernel_cmdline should be NULL. If -append "" was passed, the user wants to command line to be overwritten with "\0". > > Nope. kernel_cmdline is always a valid pointer. > > vl.c: > > [..] > if (!kernel_cmdline) { > kernel_cmdline = ""; > } Then that's on purpose. Either we change the default for everyone or not at all. But checking for "" only in the s390 machine sounds off. Alex
On 11/12/12 12:19, Alexander Graf wrote: > > On 11.12.2012, at 12:15, Christian Borntraeger wrote: > >> On 11/12/12 11:34, Alexander Graf wrote: >>> >>> On 07.12.2012, at 14:55, Jens Freimann wrote: >>> >>>> From: Christian Borntraeger <borntraeger@de.ibm.com> >>>> >>>> Since commit 967c0da73a7b0da186baba6632301d83644a570c >>>> vl.c: Avoid segfault when started with no arguments >>>> >>>> the user can specify a kernel without a command line. Lets not >>>> overwrite the default command line with \0 in that case. >>>> >>>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> >>>> Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com> >>>> --- >>>> hw/s390-virtio.c | 2 +- >>>> 1 files changed, 1 insertions(+), 1 deletions(-) >>>> >>>> diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c >>>> index ca1bb09..d77871a 100644 >>>> --- a/hw/s390-virtio.c >>>> +++ b/hw/s390-virtio.c >>>> @@ -290,7 +290,7 @@ static void s390_init(QEMUMachineInitArgs *args) >>>> stq_p(rom_ptr(INITRD_PARM_SIZE), initrd_size); >>>> } >>>> >>>> - if (rom_ptr(KERN_PARM_AREA)) { >>>> + if (rom_ptr(KERN_PARM_AREA) && strlen(kernel_cmdline)) { >>> >>> why strlen()? If no -append option was passed, kernel_cmdline should be NULL. If -append "" was passed, the user wants to command line to be overwritten with "\0". >> >> Nope. kernel_cmdline is always a valid pointer. >> >> vl.c: >> >> [..] >> if (!kernel_cmdline) { >> kernel_cmdline = ""; >> } > > Then that's on purpose. Either we change the default for everyone or not at all. But checking for "" only in the s390 machine sounds off. Ok, makes sense.
diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c index ca1bb09..d77871a 100644 --- a/hw/s390-virtio.c +++ b/hw/s390-virtio.c @@ -290,7 +290,7 @@ static void s390_init(QEMUMachineInitArgs *args) stq_p(rom_ptr(INITRD_PARM_SIZE), initrd_size); } - if (rom_ptr(KERN_PARM_AREA)) { + if (rom_ptr(KERN_PARM_AREA) && strlen(kernel_cmdline)) { /* we have to overwrite values in the kernel image, which are "rom" */ memcpy(rom_ptr(KERN_PARM_AREA), kernel_cmdline, strlen(kernel_cmdline) + 1);