Hi Ahmad,
On 26-02-06, Ahmad Fatoum wrote:
> Hi,
>
> On 2/5/26 4:45 PM, Marco Felsch wrote:
> > This adds the EFI core driver to communicate with the system
> > co-processor.
> >
> > Signed-off-by: Marco Felsch <[email protected]>
>
> Acked-by: Ahmad Fatoum <[email protected]>
>
> but see below for some nits should there be a v2.
>
> > + max_msg_len = HGS_EFI_SEP_FRAME_PREAMBLE_SZ +
> > + HGS_EFI_SEP_FRAME_POSTAMBLE_SZ +
> > + efi->coder->sep_header_hdrsize + cmd->payload_size;
> > + msg = p = xzalloc(max_msg_len);
> > + if (!msg) {
>
> xzalloc never fails.
>
> > + dev_err(dev, "No memory\n");
>
> dev_err allocates memory, so this wouldn't work anyway.
Dropped the check and the print, also for the probe() function.
> > + /* Split header from payload first for the following str-ops on buf */
> > + payload = strstr(buf, ":");
>
> Nitpick: strchr()
>
> > + if (!payload) {
> > + dev_warn(dev, "Failed to find header delim\n");
> > + return -EINVAL;
> > + }
> > +
> > + hdrlen = payload - buf;
> > + if (hdrlen > sizeof(struct hgs_efi_sep_ascii_hdr)) {
> > + dev_warn(dev, "Invalid header len detected\n");
> > + return -EINVAL;
> > + }
> > +
> > + *payload = 0;
> > + payload++;
> > +
> > + /*
> > + * Albeit the CRC is optional and the calc as a few flaws the coder may
> > + * has added it. Skip the CRC check but do the msg-id check.
> > + */
> > + p = strstr(buf, ",");
>
> Nitpick: strchr()
I applied all your findings, thanks!
Regards,
Marco
>
>
> Cheers,
> Ahmad
>
> --
> Pengutronix e.K. | |
> Steuerwalder Str. 21 | http://www.pengutronix.de/ |
> 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
>
>
--
#gernperDu
#CallMeByMyFirstName
Pengutronix e.K. | |
Steuerwalder Str. 21 | https://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |
