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

[Joe Hershberger <joe.hershberger@xxxxxx>]

Hi Lennart,

On Mon, Oct 1, 2018 at 4:56 AM, Lennart Poettering
<lennart@xxxxxxxxxxxxxx> wrote:
> 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.

OK, I changed the patch to use that instead.

> Also, could you please submit this as github PR?

Done. I created a pull request instead of sending a patch email.

Thanks,
-Joe

> 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
_______________________________________________
systemd-devel mailing list
systemd-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/systemd-devel