On 09/24/2010 02:13 PM, Brian C. Lane wrote:
On Fri, Sep 24, 2010 at 12:25:00PM -1000, David Cantrell wrote:
> Use glib functions to simplify the getPartitionsList() function in
> loader. Also use the g_strfreev() and g_strv_length() functions to
> eliminate the need for those equivalent functions in getparts.c.
> ---
A few minor comments:
1. More documentation, the previous code wasi easier to figure out
especially with the lines dealing with skipping the partition
(the check for "1" == skip extended partition wasn't obvious)
Maybe an example of /proc/partitions with an extended partition
included.
Added in some comments to explain things a bit more.
2. g_strsplit_set() max only needs to be 0, not -1
Changed.
3. check for disk == NULL at top and bail out early.
We don't want that because getPartitionsList() needs to be able to be called
with NULL and return a list of all partitions. This line handles filtering:
if (disk != NULL && !g_str_has_prefix(tokens[3], disk)) {
4. When checking the length of tokens just look for == 4, if there
aren't 4 then the following code will fail in weird ways.
This block keeps that from happening:
+ while ((j < g_strv_length(fields)) && (i < 4)) {
+ if (g_strcmp0(fields[j], "")) {
+ tokens[i++] = fields[j];
+ }
+
+ j++;
+ }
Because of the way g_strsplit_set() works, I do this to filter out the empty
fields and just count the fields with data, but then make sure we stay
within
the four field limit.
The case that I thought would not be handled were blank lines, but that is
caught fine. I ran this new version of getPartitionsList() locally
against my
/proc/partitions, both passing NULL and things like "sda" and "sdb" and it
returned the expected lists of partitions.
Looks good other than that.
Updated patch:
[PATCH] Use glib for getPartitionsList()
Use glib functions to simplify the getPartitionsList() function in
loader. Also use the g_strfreev() and g_strv_length() functions to
eliminate the need for those equivalent functions in getparts.c.
---
loader/driverdisk.c | 4 +-
loader/getparts.c | 189
+++++++++++++++++++--------------------------------
loader/getparts.h | 6 +-
loader/hdinstall.c | 4 +-
loader/loader.c | 4 +-
5 files changed, 79 insertions(+), 128 deletions(-)
diff --git a/loader/driverdisk.c b/loader/driverdisk.c
index ada8e81..2688c82 100644
--- a/loader/driverdisk.c
+++ b/loader/driverdisk.c
@@ -424,7 +424,7 @@ int loadDriverFromMedia(int class, struct
loaderData_s *loaderData,
if (part != NULL)
free(part);
- if ((nump = lenPartitionsList(part_list)) == 0) {
+ if ((nump = g_strv_length(part_list)) == 0) {
if (dir == -1)
stage = DEV_DEVICE;
else
@@ -442,7 +442,7 @@ int loadDriverFromMedia(int class, struct
loaderData_s *loaderData,
_("Back"), NULL);
if (rc == 2) {
- freePartitionsList(part_list);
+ g_strfreev(part_list);
stage = DEV_DEVICE;
dir = -1;
break;
diff --git a/loader/getparts.c b/loader/getparts.c
index c4544d7..e08e2f8 100644
--- a/loader/getparts.c
+++ b/loader/getparts.c
@@ -1,7 +1,7 @@
/*
* getparts.c - functions associated with getting partitions for a disk
*
- * Copyright (C) 1997, 1998, 1999, 2000, 2001, 2002, 2003, 2004 Red
Hat, Inc.
+ * Copyright (C) 1997-2010 Red Hat, Inc.
* All rights reserved.
*
* This program is free software; you can redistribute it and/or modify
@@ -19,6 +19,7 @@
*
* Author(s): Michael Fulbright <msf@xxxxxxxxxx>
* Jeremy Katz <katzj@xxxxxxxxxx>
+ * David Cantrell <dcantrell@xxxxxxxxxx>
*/
#include <stdio.h>
@@ -28,6 +29,7 @@
#include <errno.h>
#include <ctype.h>
#include <string.h>
+#include <glib.h>
#include "../pyanaconda/isys/log.h"
@@ -50,131 +52,80 @@ static int isPartitionName(char *pname) {
}
}
-/* return NULL terminated array of pointers to names of partitons in
- * /proc/partitions
- */
-char **getPartitionsList(char * disk) {
- FILE *f;
- int numfound = 0;
- char **rc=NULL;
-
- f = fopen("/proc/partitions", "r");
- if (!f) {
- logMessage(ERROR, "getPartitionsList: could not open
/proc/partitions");
- return NULL;
+/* Return array of the names of partitons in /proc/partitions */
+gchar **getPartitionsList(gchar *disk) {
+ guint i, j;
+ gchar *contents = NULL;
+ gchar *tokens[] = { NULL, NULL, NULL, NULL };
+ gchar **lines, **iter, **rc;
+ gsize len;
+ GError *e = NULL;
+ GSList *parts = NULL, *list = NULL;
+
+ /* read in /proc/partitions and split in to an array of lines */
+ if (!g_file_get_contents("/proc/partitions", &contents, &len, &e)) {
+ return NULL;
}
- /* read through /proc/partitions and parse out partitions */
- while (1) {
- char *tmpptr, *pptr;
- char tmpstr[4096];
-
- tmpptr = fgets(tmpstr, sizeof(tmpstr), f);
-
- if (tmpptr) {
- char *a, *b;
- int toknum = 0;
-
- a = tmpstr;
- while (1) {
- b = strsep(&a, " \n");
-
- /* if no fields left abort */
- if (!b)
- break;
-
- /* if field was empty means we hit another delimiter */
- if (!*b)
- continue;
-
- /* make sure this is a valid partition line, should start */
- /* with a numeral */
- if (toknum == 0) {
- if (!isdigit(*b))
- break;
- } else if (toknum == 2) {
- /* if size is exactly 1 then ignore it as an extended */
- if (!strcmp(b, "1"))
- break;
- } else if (toknum == 3) {
- /* this should be the partition name */
- /* now we need to see if this is the block device or */
- /* actually a partition name */
- if (!isPartitionName(b))
- break;
-
- /* make sure that either we don't care about the disk
- * or it's this one */
- if ((disk != NULL) && (strncmp(disk, b, strlen(disk))))
- break;
-
- /* we found a partition! */
- pptr = (char *) malloc(strlen(b) + 7);
- sprintf(pptr, "/dev/%s", b);
-
- if (!rc) {
- rc = (char **) malloc(2*sizeof(char *));
- rc[0] = pptr;
- rc[1] = NULL;
- } else {
- int idx;
-
- rc = (char **) realloc(rc, (numfound+2)*sizeof(char *));
- idx = 0;
- while (idx < numfound) {
- if (strcmp(pptr, rc[idx]) < 0)
- break;
-
- idx++;
- }
-
- /* move existing out of way if necessary */
- if (idx != numfound)
- memmove(rc+idx+1, rc+idx, (numfound-idx)*sizeof(char *));
-
- rc[idx] = pptr;
- rc[numfound+1] = NULL;
- }
- numfound++;
- break;
- }
- toknum++;
- }
- } else {
- break;
- }
+ if (contents == NULL) {
+ return NULL;
}
- fclose(f);
-
- return rc;
-}
-
-/* returns length of partitionlist */
-int lenPartitionsList(char **list) {
- char **part;
- int rc;
-
- if (!list) return 0;
- for (rc = 0, part = list; *part; rc++, part++);
-
- return rc;
-}
+ iter = lines = g_strsplit_set(contents, "\n", 0);
+ g_free(contents);
+
+ /* extract partition names from /proc/partitions lines */
+ while (*iter != NULL) {
+ /* split the line in to fields */
+ gchar **fields = g_strsplit_set(*iter, " ", 0);
+ i = j = 0;
+
+ if (g_strv_length(fields) > 0) {
+ /* if we're on a non-empty line, toss empty fields so we
+ * end up with the major, minor, #blocks, and name fields
+ * in positions 0, 1, 2, and 3
+ */
+ while ((j < g_strv_length(fields)) && (i < 4)) {
+ if (g_strcmp0(fields[j], "")) {
+ tokens[i++] = fields[j];
+ }
+
+ j++;
+ }
+
+ /* skip lines where:
+ * - the 'major' column is a non-digit
+ * - the '#blocks' column is '1' (indicates extended partition)
+ * - the 'name' column is not a valid partition name
+ */
+ if (isdigit(*tokens[0]) && g_strcmp0(tokens[2], "1") &&
+ isPartitionName(tokens[3])) {
+ /* if disk is specified, only return a list of partitions
+ * for that device
+ */
+ if (disk != NULL && !g_str_has_prefix(tokens[3], disk)) {
+ g_strfreev(fields);
+ iter++;
+ continue;
+ }
+
+ parts = g_slist_prepend(parts, g_strdup(tokens[3]));
+ }
+ }
-/* frees partition list */
-void freePartitionsList(char **list) {
- char **part;
+ g_strfreev(fields);
+ iter++;
+ }
- if (!list)
- return;
+ i = g_slist_length(parts);
+ rc = g_new(gchar *, i + 1);
+ rc[i] = NULL;
- for (part = list; *part; part++) {
- if (*part) {
- free(*part);
- *part = NULL;
- }
+ for (list = parts; list != NULL; list = list->next) {
+ rc[--i] = list->data;
}
- free(list);
- list = NULL;
+ g_strfreev(lines);
+ g_slist_free(parts);
+ return rc;
}
diff --git a/loader/getparts.h b/loader/getparts.h
index b672a77..52a978c 100644
--- a/loader/getparts.h
+++ b/loader/getparts.h
@@ -20,8 +20,8 @@
#ifndef GETPARTS_H
#define GETPARTS_H
-char **getPartitionsList(char * disk);
-int lenPartitionsList(char **list);
-void freePartitionsList(char **list);
+#include <glib.h>
+
+gchar **getPartitionsList(gchar * disk);
#endif
diff --git a/loader/hdinstall.c b/loader/hdinstall.c
index d52e969..39633d3 100644
--- a/loader/hdinstall.c
+++ b/loader/hdinstall.c
@@ -158,10 +158,10 @@ int promptForHardDrive(struct loaderData_s
*loaderData) {
while (1) {
/* if we're doing another pass free this up first */
if (partition_list)
- freePartitionsList(partition_list);
+ g_strfreev(partition_list);
partition_list = getPartitionsList(NULL);
- numPartitions = lenPartitionsList(partition_list);
+ numPartitions = g_strv_length(partition_list);
/* no partitions found, try to load a device driver disk for
storage */
if (!numPartitions) {
diff --git a/loader/loader.c b/loader/loader.c
index 9fccd44..2836593 100644
--- a/loader/loader.c
+++ b/loader/loader.c
@@ -478,7 +478,7 @@ void loadUpdates(struct loaderData_s *loaderData) {
part = NULL;
}
- if ((nump = lenPartitionsList(part_list)) == 0) {
+ if ((nump = g_strv_length(part_list)) == 0) {
if (dir == -1) {
stage = UPD_DEVICE;
} else {
@@ -499,7 +499,7 @@ void loadUpdates(struct loaderData_s *loaderData) {
_("Back"), NULL);
if (rc == 2) {
- freePartitionsList(part_list);
+ g_strfreev(part_list);
stage = UPD_DEVICE;
dir = -1;
break;
--
1.7.2.3
--
David Cantrell <dcantrell@xxxxxxxxxx>
Red Hat / Honolulu, HI
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
