qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Peter Maydell <peter.maydell@linaro.org>
To: "Schildbach, Wolfgang" <WSCHI@dolby.com>
Cc: qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH] Fix commandline handling for ARM semihosted executables, on Linux and BSD hosts
Date: Wed, 24 Nov 2010 12:52:59 +0000	[thread overview]
Message-ID: <AANLkTi=WttQcf8yNJVHUVhkt900voajKLRw0ZUWhXZTw@mail.gmail.com> (raw)
In-Reply-To: <F49BF8E684BC6C4188D1D63513C4CA070D2CBAB5@sparrow.dolby.net>

On 23 November 2010 15:26, Schildbach, Wolfgang <WSCHI@dolby.com> 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

  reply	other threads:[~2010-11-24 12:54 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-11-23 15:26 [Qemu-devel] [PATCH] Fix commandline handling for ARM semihosted executables, on Linux and BSD hosts Schildbach, Wolfgang
2010-11-24 12:52 ` Peter Maydell [this message]
2010-11-24 22:15   ` Schildbach, Wolfgang
  -- strict thread matches above, loose matches on Subject: below --
2010-11-24 22:19 Schildbach, Wolfgang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='AANLkTi=WttQcf8yNJVHUVhkt900voajKLRw0ZUWhXZTw@mail.gmail.com' \
    --to=peter.maydell@linaro.org \
    --cc=WSCHI@dolby.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).