[PATCH v7 8/9] cmd: efidebug: add uri device path

[Masahisa Kojima]

Hi Ilias,

On Wed, 18 Oct 2023 at 04:27, Ilias Apalodimas
<ilias.apalodimas at linaro.org> wrote:
>
> Hi Kojima-san,
>
> [...]
>
> > +                     id = (int)hextoul(argv[1], &endp);
> > +                     if (*endp != '\0' || id > 0xffff)
> > +                             return CMD_RET_USAGE;
> > +
> > +                     efi_create_indexed_name(var_name16, sizeof(var_name16),
> > +                                             "Boot", id);
> > +
> > +                     label = efi_convert_string(argv[2]);
> > +                     if (!label)
> > +                             return CMD_RET_FAILURE;
> > +                     lo.label = label;
> > +
> > +                     uridp_len = sizeof(struct efi_device_path) + strlen(argv[3]) + 1;
> > +                     fp_free = efi_alloc(uridp_len + sizeof(END));
> > +                     uridp = (struct efi_device_path_uri *)fp_free;
> > +                     uridp->dp.type = DEVICE_PATH_TYPE_MESSAGING_DEVICE;
> > +                     uridp->dp.sub_type = DEVICE_PATH_SUB_TYPE_MSG_URI;
> > +                     uridp->dp.length = uridp_len;
> > +                     if (!wget_validate_uri(argv[3])) {
> > +                             printf("ERROR: invalid URI\n");
> > +                             r = CMD_RET_FAILURE;
> > +                             goto out;
> > +                     }
>
> This needs to be moved right under label = efi_convert_string(argv[2]);
> and group all the checks together early.  You won't have to allocate and
> free the memory as well

OK.

>
> > +
> > +                     strcpy(uridp->uri, argv[3]);
> > +                     pos = (char *)uridp + uridp_len;
> > +                     memcpy(pos, &END, sizeof(END));
> > +                     fp_size += uridp_len + sizeof(END);
>
> Assign this earlier and use it on fp_free = efi_alloc(...)

OK.

Thanks,
Masahisa Kojima

>
> > +                     file_path = (struct efi_device_path *)uridp;
> > +                     argc -= 3;
>
> [...]
>
> > + *
> > + * @uri:     uri string of target file of wget
> > + * Return:   true if uri is valid, false if uri is invalid
> > + */
> > +bool wget_validate_uri(char *uri);
> > +
> >  #endif /* __NET_H__ */
> > diff --git a/net/wget.c b/net/wget.c
> > index 2087146b37..6ae2237a0a 100644
> > --- a/net/wget.c
> > +++ b/net/wget.c
> > @@ -566,3 +566,74 @@ out:
> >       return ret;
> >  }
> >  #endif
> > +
>
> Regards
> /Ilias