FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > Debian > Debian User

 
 
LinkBack Thread Tools
 
Old 09-24-2010, 10:25 PM
David Cantrell
 
Default 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 | 166 ++++++++++++++++-----------------------------------
loader/getparts.h | 6 +-
loader/hdinstall.c | 4 +-
loader/loader.c | 4 +-
5 files changed, 60 insertions(+), 124 deletions(-)

diff --git a/loader/driverdisk.c b/loader/driverdisk.c
index 951b29b..8d49209 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..bab7e65 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@redhat.com>
* Jeremy Katz <katzj@redhat.com>
+ * David Cantrell <dcantrell@redhat.com>
*/

#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,65 @@ 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;
+
+ 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, "
");
-
- /* 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);
+ iter = lines = g_strsplit_set(contents, "
", 0);
+ g_free(contents);

- return rc;
-}
+ while (*iter != NULL) {
+ gchar **fields = g_strsplit_set(*iter, " ", -1);
+ i = j = 0;

-/* returns length of partitionlist */
-int lenPartitionsList(char **list) {
- char **part;
- int rc;
+ if (g_strv_length(fields) > 0) {
+ while ((j < g_strv_length(fields)) && (i < 4)) {
+ if (g_strcmp0(fields[j], "")) {
+ tokens[i++] = fields[j];
+ }

- if (!list) return 0;
- for (rc = 0, part = list; *part; rc++, part++);
+ j++;
+ }

- return rc;
-}
+ if (isdigit(*tokens[0]) && g_strcmp0(tokens[2], "1") &&
+ isPartitionName(tokens[3])) {
+ if (disk != NULL && !g_str_has_prefix(tokens[3], disk)) {
+ g_strfreev(fields);
+ iter++;
+ continue;
+ }

-/* frees partition list */
-void freePartitionsList(char **list) {
- char **part;
+ parts = g_slist_prepend(parts, g_strdup(tokens[3]));
+ }
+ }

- if (!list)
- return;
+ g_strfreev(fields);
+ iter++;
+ }

- for (part = list; *part; part++) {
- if (*part) {
- free(*part);
- *part = NULL;
- }
+ i = g_slist_length(parts);
+ rc = g_new(gchar *, i + 1);
+ rc[i] = 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 4514404..08f255e 100644
--- a/loader/hdinstall.c
+++ b/loader/hdinstall.c
@@ -202,10 +202,10 @@ char * mountHardDrive(struct installMethod * method,
while (!done) {
/* 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 837c489..2f954e0 100644
--- a/loader/loader.c
+++ b/loader/loader.c
@@ -479,7 +479,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 {
@@ -500,7 +500,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

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
 
Old 09-25-2010, 12:13 AM
"Brian C. Lane"
 
Default Use glib for getPartitionsList()

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.

2. g_strsplit_set() max only needs to be 0, not -1

3. check for disk == NULL at top and bail out early.

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.

Looks good other than that.

--
Brian C. Lane / Anaconda Team
Port Orchard, WA (PST8PDT)
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
 
Old 10-20-2010, 04:35 PM
David Cantrell
 
Default Use glib for getPartitionsList()

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@redhat.com>
* Jeremy Katz <katzj@redhat.com>
+ * David Cantrell <dcantrell@redhat.com>
*/

#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, "
");
-
- /* 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, "
", 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@redhat.com>
Red Hat / Honolulu, HI

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
 
Old 10-20-2010, 06:34 PM
"Brian C. Lane"
 
Default Use glib for getPartitionsList()

On Wed, Oct 20, 2010 at 06:35:50AM -1000, David Cantrell wrote:
> 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.

Ack.

--
Brian C. Lane | Anaconda Team | IRC: bcl #anaconda | Port Orchard, WA (PST8PDT)
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
 

Thread Tools




All times are GMT. The time now is 09:26 AM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org