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

29 May 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"));