From a9d9b7d3cf9fa9928498273974830e9ba2002e5f Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Fri, 24 Jun 2016 14:45:05 +0100
Subject: [PATCH] p2v: Refactor into get_blockdev_size and get_blockdev_model
 functions.

This is just refactoring, but it reveals and fixes a bug too.  The
size_gb field was left as NULL when the --test-disk option was used.
Apparently passing NULL to gtk_list_store_set is fine, but just in
case I replaced it with "".

(cherry picked from commit 68ff3ffd1de7b72255ee5098c0bfef90e6236cb5)
---
 p2v/gui.c   | 40 ++++++++-----------------------------
 p2v/p2v.h   |  2 ++
 p2v/utils.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 75 insertions(+), 32 deletions(-)

diff --git a/p2v/gui.c b/p2v/gui.c
index 7414037..1bae4d8 100644
--- a/p2v/gui.c
+++ b/p2v/gui.c
@@ -869,47 +869,23 @@ populate_disks (GtkTreeView *disks_list)
                                     G_TYPE_STRING, G_TYPE_STRING);
   if (all_disks != NULL) {
     for (i = 0; all_disks[i] != NULL; ++i) {
-      CLEANUP_FREE char *size_filename = NULL;
-      CLEANUP_FREE char *model_filename = NULL;
-      CLEANUP_FREE char *size_str = NULL;
+      uint64_t size;
       CLEANUP_FREE char *size_gb = NULL;
       CLEANUP_FREE char *model = NULL;
-      uint64_t size;
 
-      if (asprintf (&size_filename, "/sys/block/%s/size",
-                    all_disks[i]) == -1) {
-        perror ("asprintf");
-        exit (EXIT_FAILURE);
-      }
-      if (g_file_get_contents (size_filename, &size_str, NULL, NULL) &&
-          sscanf (size_str, "%" SCNu64, &size) == 1) {
-        size /= 2*1024*1024; /* size from kernel is given in sectors? */
-        if (asprintf (&size_gb, "%" PRIu64, size) == -1) {
-          perror ("asprintf");
-          exit (EXIT_FAILURE);
-        }
-      }
-
-      if (asprintf (&model_filename, "/sys/block/%s/device/model",
-                    all_disks[i]) == -1) {
-        perror ("asprintf");
-        exit (EXIT_FAILURE);
-      }
-      if (g_file_get_contents (model_filename, &model, NULL, NULL)) {
-        /* Need to chomp trailing \n from the content. */
-        size_t len = strlen (model);
-        if (len > 0 && model[len-1] == '\n')
-          model[len-1] = '\0';
-      } else {
-        model = strdup ("");
+      if (all_disks[i][0] != '/') { /* not using --test-disk */
+        size = get_blockdev_size (all_disks[i]);
+        if (asprintf (&size_gb, "%" PRIu64, size) == -1)
+          error (EXIT_FAILURE, errno, "asprintf");
+        model = get_blockdev_model (all_disks[i]);
       }
 
       gtk_list_store_append (disks_store, &iter);
       gtk_list_store_set (disks_store, &iter,
                           DISKS_COL_CONVERT, TRUE,
                           DISKS_COL_DEVICE, all_disks[i],
-                          DISKS_COL_SIZE, size_gb,
-                          DISKS_COL_MODEL, model,
+                          DISKS_COL_SIZE, size_gb ? size_gb : "",
+                          DISKS_COL_MODEL, model ? model : "",
                           -1);
     }
   }
diff --git a/p2v/p2v.h b/p2v/p2v.h
index 49c97b6..40b05b8 100644
--- a/p2v/p2v.h
+++ b/p2v/p2v.h
@@ -131,6 +131,8 @@ extern mexp_h *start_remote_connection (struct config *, const char *remote_dir,
 extern const char *get_ssh_error (void);
 
 /* utils.c */
+extern uint64_t get_blockdev_size (const char *dev);
+extern char *get_blockdev_model (const char *dev);
 extern char *get_if_addr (const char *if_name);
 extern char *get_if_vendor (const char *if_name, int truncate);
 extern void wait_network_online (const struct config *);
diff --git a/p2v/utils.c b/p2v/utils.c
index 3781a8d..18c2b7c 100644
--- a/p2v/utils.c
+++ b/p2v/utils.c
@@ -20,9 +20,12 @@
 
 #include <stdio.h>
 #include <stdlib.h>
+#include <inttypes.h>
 #include <string.h>
 #include <ctype.h>
 #include <unistd.h>
+#include <errno.h>
+#include <error.h>
 #include <locale.h>
 #include <libintl.h>
 
@@ -38,6 +41,68 @@
     }                                           \
   } while (0)
 
+/**
+ * Return size of a block device, from F</sys/block/I<dev>/size>.
+ *
+ * This function always succeeds, or else exits (since we expect
+ * C<dev> to always be valid and the C<size> file to always exist).
+ */
+uint64_t
+get_blockdev_size (const char *dev)
+{
+  CLEANUP_FCLOSE FILE *fp = NULL;
+  CLEANUP_FREE char *path = NULL;
+  CLEANUP_FREE char *size_str = NULL;
+  size_t len;
+  uint64_t size;
+
+  if (asprintf (&path, "/sys/block/%s/size", dev) == -1)
+    error (EXIT_FAILURE, errno, "asprintf");
+
+  fp = fopen (path, "r");
+  if (fp == NULL)
+    error (EXIT_FAILURE, errno, "fopen: %s", path);
+  if (getline (&size_str, &len, fp) == -1)
+    error (EXIT_FAILURE, errno, "getline: %s", path);
+
+  if (sscanf (size_str, "%" SCNu64, &size) != 1)
+    error (EXIT_FAILURE, 0, "cannot parse %s: %s", path, size_str);
+
+  size /= 2*1024*1024;     /* size from kernel is given in sectors? */
+  return size;
+}
+
+/**
+ * Return model of a block device, from F</sys/block/I<dev>/device/model>.
+ *
+ * Returns C<NULL> if the file was not found.  The caller must
+ * free the returned string.
+ */
+char *
+get_blockdev_model (const char *dev)
+{
+  CLEANUP_FCLOSE FILE *fp = NULL;
+  CLEANUP_FREE char *path = NULL;
+  char *model = NULL;
+  size_t len = 0;
+  ssize_t n;
+
+  if (asprintf (&path, "/sys/block/%s/device/model", dev) == -1)
+    error (EXIT_FAILURE, errno, "asprintf");
+  fp = fopen (path, "r");
+  if (fp == NULL) {
+    perror (path);
+    return NULL;
+  }
+  if ((n = getline (&model, &len, fp)) == -1) {
+    perror (path);
+    free (model);
+    return NULL;
+  }
+  CHOMP (model, n);
+  return model;
+}
+
 /* Return contents of /sys/class/net/<if_name>/address (if found). */
 char *
 get_if_addr (const char *if_name)
-- 
2.7.4