From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:58477)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <Leon.Alrae@imgtec.com>) id 1Z0xwN-0007vc-4Y
	for qemu-devel@nongnu.org; Fri, 05 Jun 2015 16:10:08 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <Leon.Alrae@imgtec.com>) id 1Z0xwI-0003WA-5V
	for qemu-devel@nongnu.org; Fri, 05 Jun 2015 16:10:07 -0400
Received: from mailapp01.imgtec.com ([195.59.15.196]:47535)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <Leon.Alrae@imgtec.com>) id 1Z0xwI-0003Ux-07
	for qemu-devel@nongnu.org; Fri, 05 Jun 2015 16:10:02 -0400
Message-ID: <55720210.7010200@imgtec.com>
Date: Fri, 5 Jun 2015 21:09:52 +0100
From: Leon Alrae <leon.alrae@imgtec.com>
MIME-Version: 1.0
References: <1432656234-9791-1-git-send-email-leon.alrae@imgtec.com>
	<1432656234-9791-3-git-send-email-leon.alrae@imgtec.com>
	<CAFEAcA-RCY_Z7exFLk4f9k=ODbFXBYov+36m1vqmw3MD_2khvg@mail.gmail.com>
In-Reply-To: <CAFEAcA-RCY_Z7exFLk4f9k=ODbFXBYov+36m1vqmw3MD_2khvg@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH v4 2/2] semihosting: add
	--semihosting-config arg sub-argument
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: Liviu Ionescu <ilg@livius.net>, Christopher Covington <christopher.covington@linaro.org>, QEMU Developers <qemu-devel@nongnu.org>, Matthew Fortune <matthew.fortune@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