The sole purpose of getDeviceType() is to parse a file that
contains one integer (and a newline character). Well, we already
have a function for that: virFileReadValueInt(). Use the latter
and drop the former.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/storage/storage_util.c | 75 ++++++++------------------------------
 1 file changed, 15 insertions(+), 60 deletions(-)

diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index bf8de2475d..7243308a02 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -3847,62 +3847,6 @@ getBlockDevice(uint32_t host,
 }
 
 
-/* Function to check if the type file in the given sysfs_path is a
- * Direct-Access device (i.e. type 0).  Return -1 on failure, type of
- * the device otherwise.
- */
-static int
-getDeviceType(uint32_t host,
-              uint32_t bus,
-              uint32_t target,
-              uint32_t lun,
-              int *type)
-{
-    char typestr[3];
-    char *gottype, *p;
-    FILE *typefile;
-    g_autofree char *type_path = NULL;
-
-    type_path = g_strdup_printf("/sys/bus/scsi/devices/%u:%u:%u:%u/type", host,
-                                bus, target, lun);
-
-    typefile = fopen(type_path, "r");
-    if (typefile == NULL) {
-        virReportSystemError(errno,
-                             _("Could not find typefile '%1$s'"),
-                             type_path);
-        /* there was no type file; that doesn't seem right */
-        return -1;
-    }
-
-    gottype = fgets(typestr, 3, typefile);
-    VIR_FORCE_FCLOSE(typefile);
-
-    if (gottype == NULL) {
-        virReportSystemError(errno,
-                             _("Could not read typefile '%1$s'"),
-                             type_path);
-        /* we couldn't read the type file; have to give up */
-        return -1;
-    }
-
-    /* we don't actually care about p, but if you pass NULL and the last
-     * character is not \0, virStrToLong_i complains
-     */
-    if (virStrToLong_i(typestr, &p, 10, type) < 0) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("Device type '%1$s' is not an integer"),
-                       typestr);
-        /* Hm, type wasn't an integer; seems strange */
-        return -1;
-    }
-
-    VIR_DEBUG("Device type is %d", *type);
-
-    return 0;
-}
-
-
 /*
  * Process a Logical Unit entry from the scsi host device directory
  *
@@ -3921,18 +3865,29 @@ processLU(virStoragePoolObj *pool,
 {
     int retval = -1;
     int device_type;
+    int rc;
     g_autofree char *block_device = NULL;
 
     VIR_DEBUG("Processing LU %u:%u:%u:%u",
               host, bus, target, lun);
 
-    if (getDeviceType(host, bus, target, lun, &device_type) < 0) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("Failed to determine if %1$u:%2$u:%3$u:%4$u is a Direct-Access LUN"),
-                       host, bus, target, lun);
+    if ((rc = virFileReadValueInt(&device_type,
+                                  "/sys/bus/scsi/devices/%u:%u:%u:%u/type",
+                                  host, bus, target, lun)) < 0) {
+
+        /* Report an error if file doesn't exist. Appropriate
+         * error was reported otherwise. */
+        if (rc == -2) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("Failed to determine if %1$u:%2$u:%3$u:%4$u is a Direct-Access LUN"),
+                           host, bus, target, lun);
+        }
+
         return -1;
     }
 
+    VIR_DEBUG("Device type is %d", device_type);
+
     /* We don't create volumes for devices other than disk and cdrom
      * devices, but finding a device that isn't one of those types
      * isn't an error, either. */
-- 
2.41.0

On a Tuesday in 2023, Michal Privoznik wrote:
>The sole purpose of getDeviceType() is to parse a file that
>contains one integer (and a newline character). Well, we already
>have a function for that: virFileReadValueInt(). Use the latter
>and drop the former.
>
>Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>---
> src/storage/storage_util.c | 75 ++++++++------------------------------
> 1 file changed, 15 insertions(+), 60 deletions(-)
>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano