Re: [PATCH] udev: Allow acpi_index and index to be "0"

[Lennart Poettering <lennart@xxxxxxxxxxxxxx>]

On Fr, 28.09.18 15:46, Joe Hershberger (joe.hershberger@xxxxxx) wrote:

> 0 can be a valid index returned by the BIOS, so allow that literal while
> still checking for 0 as a failed conversion by strtoul(). Also, unsigned
> long cannot be negative, so don't misleadingly check for less than 0.
> 
> Signed-off-by: Joe Hershberger <joe.hershberger@xxxxxx>
> ---
>  src/udev/udev-builtin-net_id.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/udev/udev-builtin-net_id.c b/src/udev/udev-builtin-net_id.c
> index 5341d3788..338a2d4c4 100644
> --- a/src/udev/udev-builtin-net_id.c
> +++ b/src/udev/udev-builtin-net_id.c
> @@ -232,7 +232,7 @@ static int dev_pci_onboard(struct udev_device *dev, struct netnames *names) {
>                  return -ENOENT;
>  
>          idx = strtoul(attr, NULL, 0);
> -        if (idx <= 0)
> +        if (idx == 0 && strcmp("0", attr))
>                  return -EINVAL;

So, strtoul() error checking is messy. Let's just clean this up
properly: we have a call safe_atolu() in parse-util.h in place already
that we use preferably over raw strtoul() and that returns errors in a
more sane form. Hence, could you please rework your patch to that this
uses safe_atolu() instead of strtoul()? That would make it shorter,
cleaner and more elegant.

Also, could you please submit this as github PR?

https://github.com/systemd/systemd/pulls

Lennart

-- 
Lennart Poettering, Red Hat
_______________________________________________
systemd-devel mailing list
systemd-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/systemd-devel