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

["Justin He (Arm Technology China)" <Justin.He@xxxxxxx>]

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

> 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

>  #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))

> +             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));
                    ^~~~~~~
>  }
>
>  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