On Thu, Jun 23, 2016 at 08:26:06AM -0400, John Ferlan wrote:
On 06/21/2016 12:05 PM, Ján Tomko wrote:
In the unlikely case the iSCSI session path exists, but does not contain an entry starting with "target", we would silently use an initialized value.
Rewrite the function to correctly report errors. --- src/storage/storage_backend_iscsi.c | 38 +++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-)
diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c index bccfba3..876c14c 100644 --- a/src/storage/storage_backend_iscsi.c +++ b/src/storage/storage_backend_iscsi.c @@ -90,7 +90,7 @@ static int virStorageBackendISCSIGetHostNumber(const char *sysfs_path, uint32_t *host) { - int retval = 0; + int ret = -1; DIR *sysdir = NULL; struct dirent *dirent = NULL; int direrr; @@ -104,26 +104,33 @@ virStorageBackendISCSIGetHostNumber(const char *sysfs_path, if (sysdir == NULL) { virReportSystemError(errno, _("Failed to opendir path '%s'"), sysfs_path); - retval = -1; - goto out; + goto cleanup; }
while ((direrr = virDirRead(sysdir, &dirent, sysfs_path)) > 0) { if (STREQLEN(dirent->d_name, "target", strlen("target"))) {
While we're here and changing, could use STRPREFIX
I'll send a separate patch for that.
- if (sscanf(dirent->d_name, - "target%u:", host) != 1) { - VIR_DEBUG("Failed to parse target '%s'", dirent->d_name); - retval = -1; - break; + if (sscanf(dirent->d_name, "target%u:", host) == 1) { + ret = 0; + goto cleanup; + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to parse target '%s'"), dirent->d_name); + goto cleanup; } } } - if (direrr < 0) - retval = -1;
+ if (direrr == 0) {
Should this be <= ?
If virDirRead() returns -1, then we don't return with an error...
virDirRead does report errors.
If virDirRead() returns 0, then we've gone through the entire directory without finding an entry that starts with target - seems that would be a system configuration error and I assume errno would be ENOENT
Actually, errno would probably be 0. I'll fix the error in v2. Jan
ACK with obvious adjustments -
John