[libvirt] [PATCH v12] add 802.1Qbh and 802.1Qbg handling
Jim Meyering
jim at meyering.net
Sat May 29 08:08:11 UTC 2010
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"));
More information about the libvir-list
mailing list