Re: [Qemu-devel] [PATCH v4 2/2] semihosting: add --semihosting-config arg sub-argument

[Leon Alrae <leon.alrae@imgtec.com>]

On 05/06/15 16:23, Peter Maydell wrote:
> On 26 May 2015 at 17:03, Leon Alrae <leon.alrae@imgtec.com> wrote:
>> --- a/target-arm/arm-semi.c
>> +++ b/target-arm/arm-semi.c
>> @@ -27,6 +27,7 @@
>>  #include <time.h>
>>
>>  #include "cpu.h"
>> +#include "exec/semihost.h"
>>  #ifdef CONFIG_USER_ONLY
>>  #include "qemu.h"
>>
>> @@ -440,10 +441,7 @@ uint32_t do_arm_semihosting(CPUARMState *env)
>>              input_size = arg1;
>>              /* Compute the size of the output string.  */
>>  #if !defined(CONFIG_USER_ONLY)
>> -            output_size = strlen(ts->boot_info->kernel_filename)
>> -                        + 1  /* Separating space.  */
>> -                        + strlen(ts->boot_info->kernel_cmdline)
>> -                        + 1; /* Terminating null byte.  */
>> +            output_size = strlen(semihosting_get_cmdline()) + 1;
> 
> It looks like semihosting_get_cmdline() can return NULL,
> in which case this will blow up, I think.

semihosting_get_cmdline() returns NULL if neither semihosting args nor
-kernel have been specified. As far as I can tell existing
implementation may also blow up if kernel_filename is NULL, so we retain
the same behaviour. Besides, it's not clear to me how the
TARGET_SYS_GET_CMDLINE should behave if cmdline is not available,
whether should return -1 or pass an empty string to the guest. For me
this looks like a separate issue, not much related to this patch series.

Thanks,
Leon