Skip to contentSkip to conversation

libfdisk: fix get_partition_unused_primary()

PATCH Applied Boris Egorov 2 participants 2 messages Nov 14, 2014 Util-Linux package

lore
Share & tools
lore

Boris Egorov

author
#0 lore raw

PATCH 1/1 libfdisk: fix get_partition_unused_primary()

Was:
Mentioned function returns -1 if adding of primary partition is impossible. Caller treats this value as size_t (res variable) and then compares it for negative values, totally ignoring errors.

Becomes:
Now function takes address to variable and fills it with partition number. Caller treats return value as int and use it appropriately.

Signed-off-by Boris Egorov <[email protected]>
---
 libfdisk/src/dos.c | 46 +++++++++++++++++++++++++---------------------
 1 file changed, 25 insertions(+), 21 deletions(-)
libfdisk/src/dos.c +25 -21
index b4456a9..0edb5fd 100644
@@ -186,24 +186,28 @@ static int is_cleared_partition(struct dos_partition *p)
 }
 
 static int get_partition_unused_primary(struct fdisk_context *cxt,
-					struct fdisk_partition *pa)
+					struct fdisk_partition *pa,
+					size_t *partno)
 {
-	size_t org = cxt->label->nparts_max, n;
+	size_t org, n;
 	int rc;
 
+	assert(cxt);
+	assert(cxt->label);
+	assert(partno);
+
+	org = cxt->label->nparts_max;
+
 	cxt->label->nparts_max = 4;
 	rc = fdisk_partition_next_partno(pa, cxt, &n);
 	cxt->label->nparts_max = org;
 
-	switch (rc) {
-	case 1:
+	if (rc == 1) {
 		fdisk_info(cxt, _("All primary partitions have been defined already."));
 		return -1;
-	case 0:
-		return n;
-	default:
-		return rc;
 	}
+	*partno = rc == 0 ? n : rc;
+	return 0;
 }
 
 static int seek_sector(struct fdisk_context *cxt, sector_t secno)

@@ -1464,8 +1468,8 @@ static int dos_add_partition(struct fdisk_context *cxt,
 			fdisk_warnx(cxt, _("Extended partition already exists."));
 			return -EINVAL;
 		}
-		res = get_partition_unused_primary(cxt, pa);
-		if (res >= 0) {
+		rc = get_partition_unused_primary(cxt, pa, &res);
+		if (rc == 0) {
 			rc = add_partition(cxt, res, pa);
 			goto done;
 		}

@@ -1473,8 +1477,8 @@ static int dos_add_partition(struct fdisk_context *cxt,
 	/* pa specifies start, but outside extended partition */
 	} else if (pa && fdisk_partition_has_start(pa) && l->ext_offset) {
 		DBG(LABEL, ul_debug("DOS: pa template %p: add primary", pa));
-		res = get_partition_unused_primary(cxt, pa);
-		if (res >= 0) {
+		rc = get_partition_unused_primary(cxt, pa, &res);
+		if (rc == 0) {
 			rc = add_partition(cxt, res, pa);
 			goto done;
 		}

@@ -1531,8 +1535,8 @@ static int dos_add_partition(struct fdisk_context *cxt,
 	} else if (cxt->label->nparts_max >= MAXIMUM_PARTS) {
 		fdisk_info(cxt, _("All logical partitions are in use. "
 				  "Adding a primary partition."));
-		res = get_partition_unused_primary(cxt, pa);
-		if (res >= 0)
+		rc = get_partition_unused_primary(cxt, pa, &res);
+		if (rc == 0)
 			rc = add_partition(cxt, res, pa);
 	} else {
 		char hint[BUFSIZ];

@@ -1541,8 +1545,8 @@ static int dos_add_partition(struct fdisk_context *cxt,
 
 		/* the default layout for scripts is to create primary partitions */
 		if (cxt->script) {
-			res = get_partition_unused_primary(cxt, pa);
-			if (res >= 0)
+			rc = get_partition_unused_primary(cxt, pa, &res);
+			if (rc == 0)
 				rc = add_partition(cxt, res, pa);
 			goto done;
 		}

@@ -1573,16 +1577,16 @@ static int dos_add_partition(struct fdisk_context *cxt,
 		fdisk_free_ask(ask);
 
 		if (c == 'p') {
-			res = get_partition_unused_primary(cxt, pa);
-			if (res >= 0)
+			rc = get_partition_unused_primary(cxt, pa, &res);
+			if (rc == 0)
 				rc = add_partition(cxt, res, pa);
 			goto done;
 		} else if (c == 'l' && l->ext_offset) {
 			rc = add_logical(cxt, pa, &res);
 			goto done;
 		} else if (c == 'e' && !l->ext_offset) {
-			res = get_partition_unused_primary(cxt, pa);
-			if (res >= 0) {
+			rc = get_partition_unused_primary(cxt, pa, &res);
+			if (rc == 0) {
 				struct fdisk_partition *xpa = NULL;
 				struct fdisk_parttype *t;
 

@@ -1607,7 +1611,7 @@ static int dos_add_partition(struct fdisk_context *cxt,
 done:
 	if (rc == 0) {
 		cxt->label->nparts_cur++;
-		if (partno && res >= 0)
+		if (partno)
 			*partno = res;
 	}
 	return rc;
Signature · 1 line
2.1.1
maintainer Applied
#1 lore raw
Quoted 2 lines · Boris Egorov

Now function takes address to variable and fills it with partition number. Caller treats return value as int and use it appropriately.

Yep, it's better.

Quoted 1 line

static int get_partition_unused_primary(struct fdisk_context *cxt,

...

libfdisk/src/dos.c static int is_cleared_partition(struct dos_partition *p) · line 209

Quoted 1 line
libfdisk/src/dos.c +1 -0
+	*partno = rc == 0 ? n : rc;

        ^^^^^^^^^^^

I have replaced this line with

if (rc == 0)

    *partno = n;

it seems we don't have to mix partno and return codes here at all.

libfdisk/src/dos.c static int is_cleared_partition(struct dos_partition *p) · line 210

Quoted 1 line
libfdisk/src/dos.c +1 -0
+	return 0;

return rc;

Applied, thanks!

Karel

Signature · 2 lines
 Karel Zak  <[email protected]>
 http://karelzak.blogspot.com