Re: [Minios-devel] [UUNIKRAFT PATCH] plat/kvm/arm: Add image name as the first argument

[Sharan Santhanam <sharan.santhanam@xxxxxxxxx>]

On 8/15/19 5:09 AM, Justin He (Arm Technology China) wrote:
> Hi Sharan, please find my comments below
>
> > -----Original Message-----
> > From: Sharan Santhanam <sharan.santhanam@xxxxxxxxx>
> > Sent: 2019年8月14日 22:29
> > To: minios-devel@xxxxxxxxxxxxx
> > Cc: Justin He (Arm Technology China) <Justin.He@xxxxxxx>; Sharan
> > Santhanam <sharan.santhanam@xxxxxxxxx>
> > Subject: [UUNIKRAFT PATCH] plat/kvm/arm: Add image name as the first
> Nit, s/ UUNIKRAFT/ UNIKRAFT
Yes I fixed it already in the v2 which was in the mailing list.

> > argument
> >
> > While setting up the command line arguments to an application, the user
> > argument start at index 0 instead of having the application name
> > followed by the user arguments.
> >
> > Signed-off-by: Sharan Santhanam <sharan.santhanam@xxxxxxxxx>
> > ---
> >   plat/kvm/arm/setup.c | 16 +++++++++++-----
> >   1 file changed, 11 insertions(+), 5 deletions(-)
> >
> > diff --git a/plat/kvm/arm/setup.c b/plat/kvm/arm/setup.c
> > index b8148f9c..02f89664 100644
> > --- a/plat/kvm/arm/setup.c
> > +++ b/plat/kvm/arm/setup.c
> > @@ -18,6 +18,7 @@
> >    * NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
> >    * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> >    */
> > +#include <uk/config.h>
> Why should this config.h be needed in this source file?
> Is this intended for using CONFIG_UK_NAME, but I didn't see this including in
> Xen/linuxu or x86
Yes. Maybe it get included from the one of the existing headers. I would 
keep it as there is a dependency on this header.
> >   #include <libfdt.h>
> >   #include <sections.h>
> >   #include <kvm/console.h>
> > @@ -32,6 +33,7 @@ struct kvmplat_config _libkvmplat_cfg = { 0 };
> >
> >   #define MAX_CMDLINE_SIZE 1024
> >   static char cmdline[MAX_CMDLINE_SIZE];
> > +static const char *appname = CONFIG_UK_NAME;
> >
> >   smcc_psci_callfn_t smcc_psci_call;
> >
> > @@ -181,22 +183,26 @@ static void _dtb_get_cmdline(char *cmdline,
> > size_t maxlen)
> >        if (!fdtcmdline || (len <= 0))
> >                goto enocmdl;
> >
> > -     strncpy(cmdline, fdtcmdline, MIN(maxlen, (unsigned int) len));
> > +     /* adding a white space between the app name and the arguments
> > */
> > +     if (likely(maxlen >= len))
> My compiler complains:
> /root/hj/UK/unikraft_upstream/unikraft/plat/kvm/arm/setup.c: In function 
> ‘_dtb_get_cmdline’:
> /root/hj/UK/unikraft_upstream/unikraft/plat/kvm/arm/setup.c:186:20: warning: 
> comparison between signed and unsigned integer expressions [-Wsign-compare]
>    if (likely(maxlen >= len))
Sure I will fix it in v3

> > +             maxlen = len;
> > +     else
> > +             uk_pr_err("Command line too long, truncated\n");
> > +
> > +     strncpy(cmdline, fdtcmdline, maxlen);
> >        /* ensure null termination */
> > -     cmdline[((unsigned int) len - 1) <= (maxlen - 1) ?
> > -             ((unsigned int) len - 1) : (maxlen - 1)] = '\0';
> > +     cmdline[maxlen - 1] = '\0';
> >
> >        uk_pr_info("Command line: %s\n", cmdline);
> >        return;
> >
> >   enocmdl:
> >        uk_pr_info("No command line found\n");
> > -     strcpy(cmdline, CONFIG_UK_NAME);
> >   }
> >
> >   static void _libkvmplat_entry2(void *arg __attribute__((unused)))
> >   {
> > -     ukplat_entry_argp(NULL, (char *)cmdline, strlen(cmdline));
> > +     ukplat_entry_argp(appname, (char *)cmdline, strlen(cmdline));
> My compiler complains:
> /root/hj/UK/unikraft_upstream/unikraft/plat/kvm/arm/setup.c: In function 
> ‘_libkvmplat_entry2’:
> /root/hj/UK/unikraft_upstream/unikraft/plat/kvm/arm/setup.c:204:20: warning: passing argument 1 of 
> ‘ukplat_entry_argp’ discards ‘const’ qualifier from pointer target type 
> [-Wdiscarded-qualifiers]
>    ukplat_entry_argp(appname, (char *)cmdline, strlen(cmdline));
Fix it in v3

>                      ^~~~~~~
> >   }
> >
> >   void _libkvmplat_start(void *dtb_pointer)
> > --
> > 2.20.1
> IMPORTANT NOTICE: The contents of this email and any attachments are 
> confidential and may also be privileged. If you are not the intended recipient, 
> please notify the sender immediately and do not disclose the contents to any 
> other person, use it for any purpose, or store or copy the information in any 
> medium. Thank you.
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel