Re: [PATCH v12] add 802.1Qbh and 802.1Qbg handling

[Jim Meyering <jim@xxxxxxxxxxxx>]

Stefan Berger wrote:
...
> V12:
>  - Addressing Jim Meyering's comments to v11
>  - requiring mac address to the vpDisassociateProfileId() function to
>    pass it further to the 802.1Qbg disassociate part (802.1Qbh untouched)

Thanks for enduring so many iterations.
...
> +static uint32_t
> +getLldpadPid(void) {
> +    int fd;
> +    uint32_t pid = 0;
> +
> +    fd = open(LLDPAD_PID_FILE, O_RDONLY);
> +    if (fd >= 0) {
> +        char buffer[10];
> +        char *endptr;
> +
> +        if (saferead(fd, buffer, sizeof(buffer)) <= sizeof(buffer)) {
> +            unsigned int res;
> +
> +            if (virStrToLong_ui(buffer, &endptr, 10, &res) &&
> +                (endptr == NULL || c_isspace(*endptr)))
> +                macvtapError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                             _("error parsing pid of lldpad"));
> +            else
> +                pid = res;
> +        }

That new && (...) conjunct is wrong, since it makes the code
check endptr only when parsing fails.
Writing || !(...) would have worked but is harder to read than...

How about this, reversing the if/else blocks?
Also, this adds a test to diagnose a PID with value 0 as invalid
(which would indicate failure with no diagnostic)
and adjusts the diagnostic accordingly, since that is not a parse error:

            if (virStrToLong_ui(buffer, &endptr, 10, &res) == 0
                && (endptr == NULL || c_isspace(*endptr))
                && res != 0)
                pid = res;
            else
                macvtapError(VIR_ERR_INTERNAL_ERROR, "%s",
                             _("invalid lldpad PID"));

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list