On Wed, 2022-12-21 at 15:11 +0100, Daniel Kiper wrote:
> > + if (!port && grub_memcmp (name, "mmio,", sizeof ("mmio,") - 1) == 0
>
> I think grub_strncmp() in string context would be more appropriate.
> Please replace grub_memcmp() with grub_strncmp() where possible.
Same comment as patch 3, this is a pre-existing construct (look a few
lines above in the original code:
if (!port && grub_memcmp (name, "port", sizeof ("port") - 1) == 0
&& grub_isxdigit (name [sizeof ("port") - 1]))
{
I'll fix them all in the respun patch.
> > + && grub_isxdigit (name [sizeof ("mmio,") - 1]))
> > + {
> > + const char *p1, *p = &name[sizeof ("mmio,") - 1];
> > + grub_addr_t addr = grub_strtoul (p, &p1, 16);
>
> You blindly assume grub_strtoul() will not fail. It is not true.
> Please take a look at commit ac8a37dda (net/http: Allow use of
> non-standard TCP/IP ports) how properly detect grub_strtoul() failures.
Indeed, that's not great, I'll fix.
> > + unsigned int acc_size = 1;
> > + unsigned int nlen = p1 - p;
>
> Please add empty line here.
>
> > + if (p1[0] == '.')
> > + switch(p1[1])
> > + {
> > + case 'w':
> > + acc_size = 2;
> > + break;
> > + case 'l':
> > + acc_size = 3;
> > + break;
> > + case 'q':
> > + acc_size = 4;
> > + break;
>
> Does not compiler complain there is no default here? I think I saw such
> warnings somewhere when it is missing.
I'm pretty sure it didn't but it might depend on a specific gcc
version, I'll fix.
Cheers,
Ben.
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
