RE: [Qemu-devel] [PATCH] Fix commandline handling for ARM semihosted executables, on Linux and BSD hosts

[Schildbach, Wolfgang]

OK, I declare defeat :-) I'll followup with a patch that special-cases an empty 
command line.

- Wolfgang
 

-----Original Message-----
From: Peter Maydell [mailto:address@hidden 
Sent: Wednesday, November 24, 2010 1:53 PM
To: Schildbach, Wolfgang
Cc: address@hidden
Subject: Re: [Qemu-devel] [PATCH] Fix commandline handling for ARM semihosted 
executables, on Linux and BSD hosts

On 23 November 2010 15:26, Schildbach, Wolfgang <address@hidden> wrote:
> When running an ARM semihosted executable on a linux machine, the 
> command line is not delivered to the guest (see 
> https://bugs.launchpad.net/qemu/+bug/673613).

I've tested this, and it does work; however I don't think the code you have to 
handle the "empty command line" case is right.

> +                if (!host_cmdline_len) {
> +                    /* is arcg==0 even a possibility? */
> +                    arm_cmdline_buffer[0] = 0;
> +                    host_cmdline_len=1;
> +                }

To answer the question in the comment:
You can certainly start a native binary with argc==0 (ie an argv with only the 
terminating NULL) using execv(). The qemu loader_exec() function similarly 
separates the binary to be started from the argv array in its API. As it 
happens the command line parsing in linux-user/main.c doesn't provide the user 
with a way to give an empty array to loader_exec(), but I think it's more 
robust to just handle the special case here rather than impose an undocumented 
restriction on use of loader_exec(). (After all ARM semihosting is definitely a 
minority sport and this isn't exactly a performance critical bit of code :-))

In any case, I think that your handling for this case is being done too late. 
By this point you have already done the "is the buffer big enough" check and 
locked the user buffers with a host_cmdline_len of zero even though you're now 
writing a byte here.

My suggestion would be just to handle this case early, by putting something 
like this before the 'command line too long' test:

 if (host_cmdline_len == 0) {
     /* Simpler to treat the "empty command line" case specially */
     if (arm_cmdline_len < 1) {
         return -1;
     }
     arm_cmdline_buffer = lock_user(VERIFY_WRITE, ARG(0), 1, 0);
     arm_cmdline_buffer[0] = 0;
     SET_ARG(1, 0);
     unlock_user(arm_cmdline_buffer, ARG(0), 1);
     return 0;
 }

-- PMM