Skip to content

Commit

Permalink
util: refactor function to simplify and remove label
Browse files Browse the repository at this point in the history
Once the DIR* in virPCIGetName() was made g_autoptr, the cleanup:
label just had a "return ret;", but the rest of the function was more
compilcated than it needed to be, doing funky things with the value of
ret inside multi-level conditionals and a while loop that might exit
early via a break with ret == 0 or exit early via a goto cleanup with
ret == -1.

It really didn't need to be nearly as complicated. After doing the
trivial replacements of "goto cleanup" with appropriate direct
returns, it became obvious that:

1) the outermost level of the nested conditional at the end of the
   function ("if (ret < 0)") was now redundant, since ret is now
   *always* < 0 by that point (otherwise the function has returned).

2) by switching the sense of the next level of the conditional (making
   it "if (!physPortID)", the "else" (which is now just "return 0;"
   becomes the "if", and the new "else" no longer needs to be inside
   the conditional.

3) the value of firstEntryName can be moved into *netname with
   g_steal_pointer()

Once that is all done, ret is no longer used and can be removed.

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
  • Loading branch information
Laine Stump committed Nov 3, 2020
1 parent d4f071d commit 77401d5
Showing 1 changed file with 20 additions and 29 deletions.
49 changes: 20 additions & 29 deletions src/util/virpci.c
Expand Up @@ -2384,7 +2384,6 @@ virPCIGetNetName(const char *device_link_sysfs_path,
{
g_autofree char *pcidev_sysfs_net_path = NULL;
g_autofree char *firstEntryName = NULL;
int ret = -1;
g_autoptr(DIR) dir = NULL;
struct dirent *entry = NULL;
size_t i = 0;
Expand All @@ -2399,8 +2398,7 @@ virPCIGetNetName(const char *device_link_sysfs_path,

if (virDirOpenQuiet(&dir, pcidev_sysfs_net_path) < 0) {
/* this *isn't* an error - caller needs to check for netname == NULL */
ret = 0;
goto cleanup;
return 0;
}

while (virDirRead(dir, &entry, pcidev_sysfs_net_path) > 0) {
Expand All @@ -2411,7 +2409,7 @@ virPCIGetNetName(const char *device_link_sysfs_path,
g_autofree char *thisPhysPortID = NULL;

if (virNetDevGetPhysPortID(entry->d_name, &thisPhysPortID) < 0)
goto cleanup;
return -1;

/* if this one doesn't match, keep looking */
if (STRNEQ_NULLABLE(physPortID, thisPhysPortID)) {
Expand All @@ -2431,34 +2429,27 @@ virPCIGetNetName(const char *device_link_sysfs_path,
}

*netname = g_strdup(entry->d_name);

ret = 0;
break;
return 0;
}

if (ret < 0) {
if (physPortID) {
if (firstEntryName) {
/* we didn't match the provided phys_port_id, but this
* is probably because phys_port_id isn't implemented
* for this NIC driver, so just return the first
* (probably only) netname we found.
*/
*netname = firstEntryName;
firstEntryName = NULL;
ret = 0;
} else {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Could not find network device with "
"phys_port_id '%s' under PCI device at %s"),
physPortID, device_link_sysfs_path);
}
} else {
ret = 0; /* no netdev at the given index is *not* an error */
}
if (!physPortID)
return 0;

if (firstEntryName) {
/* we didn't match the provided phys_port_id, but this
* is probably because phys_port_id isn't implemented
* for this NIC driver, so just return the first
* (probably only) netname we found.
*/
*netname = g_steal_pointer(&firstEntryName);
return 0;
}
cleanup:
return ret;

virReportError(VIR_ERR_INTERNAL_ERROR,
_("Could not find network device with "
"phys_port_id '%s' under PCI device at %s"),
physPortID, device_link_sysfs_path);
return -1;
}

int
Expand Down

0 comments on commit 77401d5

Please sign in to comment.