[parted-devel] [PATCH 1/2] parted: mkpart: subtract one sector fron end if IEC unit is used

Jim Meyering jim at meyering.net
Fri Oct 28 17:21:59 UTC 2011


Petr Uzel wrote:
> Before, if the user specified start and end in mkpart command using
> IEC units, parted created a partition that starts and ends exactly on
> these positions. With such behavior, it is impossible to create
> partitions as follows: 1MiB-2MiB, 2MiB-3MiB - parted would complain
> that it cannot create the second partition, because the first one
> occupied sectors 2048-4096 and the second one sectors 4096-3072,
> so they would overlap at sector 4096.
>
> With this patch, if the user uses IEC units to specify end of the
> partition, parted creates the partition which ends one sector before
> the specified position.
>
> See also
> https://lists.gnu.org/archive/html/bug-parted/2011-10/msg00009.html
>
> * parted/ui.c (command_line_get_sector): Add parameter to retrieve
> raw input from user.
> * parted/ui.h (command_line_get_sector): Adjust prototype of function.
> * parted/parted.c (_adjust_end_if_iec): New function.
> (_rstrip_string): New function.
> (_string_ends_with_iec_unit): New function.
> (do_mkpart): Call _adjust_end_if_iec(). Use new parameter of
> command_line_get_sector function.
> (do_rescue): Adjust call to command_line_get_sector.
> * NEWS: Mention the changed behavior.

Thanks for this improvement.
I've fixed a few minor problems and added a test to highlight
one more that I spotted in review.  That test is currently failing.
Would you please suggest a patch on top of these 4 (which I plan
to merge into yours) that fixes the new failing test?

Jim


>From a5add74eed16d0849f6c2c3e5e80d5e5de9768cb Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering at redhat.com>
Date: Thu, 27 Oct 2011 17:19:51 +0200
Subject: [PATCH 1/4] misc corrections

- function renaming
- "*" pointer syntax
- s/isspace/c_isblank/ so that parsing is locale-independent
- avoid a const-discard warning.
---
 parted/parted.c |   13 +++++++------
 1 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/parted/parted.c b/parted/parted.c
index 6f90cab..07f42c6 100644
--- a/parted/parted.c
+++ b/parted/parted.c
@@ -53,6 +53,7 @@
 #include <string.h>
 #include <unistd.h>
 #include <limits.h>
+#include "c-ctype.h"
 #include "xalloc.h"

 #ifdef ENABLE_MTRACE
@@ -524,14 +525,14 @@ error:
 }

 /* Strip whitespace character from end of the string, in place */
-void _rstrip_string(char* str)
+void _strip_trailing_spaces(char *str)
 {
         if (!str)
                 return;

-        int i;
+        size_t i;
         for (i = strlen(str) - 1; i; i--)
-                if (isspace(str[i]))
+                if (c_isblank(str[i]))
                     str[i]='\0';
         else
                 break;
@@ -539,13 +540,13 @@ void _rstrip_string(char* str)

 /* Return true, if str ends with [kMGT]iB, i.e. the IEC unit */
 static bool
-_string_ends_with_iec_unit(const char* str)
+_string_ends_with_iec_unit(const char *str)
 {
         /* 3 characters for the IEC unit and at least 1 digit */
         if (!str || strlen(str) < 4)
                 return false;

-        char *last3 = str + strlen(str) - 3;
+        char const *last3 = str + strlen(str) - 3;
         if (!strcmp(last3, "kiB") || !strcmp(last3, "MiB") ||
             !strcmp(last3, "GiB") || !strcmp(last3, "TiB"))
                 return true;
@@ -574,7 +575,7 @@ _adjust_end_if_iec (PedSector* start, PedSector* end,
         if (*start == *end)
                 return;

-        _rstrip_string(end_input);
+        _strip_trailing_spaces(end_input);
         PedUnit unit = ped_unit_get_default();
         if (_string_ends_with_iec_unit(end_input) ||
             (unit == PED_UNIT_KIBIBYTE) || (unit == PED_UNIT_MEBIBYTE) ||
--
1.7.7.1.476.g9890


>From f4cc9856e5c3b31806a48ba1dd9b3c599f44a4e8 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering at redhat.com>
Date: Thu, 27 Oct 2011 17:38:40 +0200
Subject: [PATCH 2/4] avoid array bounds error

* parted/parted.c (_strip_trailing_spaces): Don't deref str[-1]
for an empty string.
---
 parted/parted.c |   12 ++++--------
 1 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/parted/parted.c b/parted/parted.c
index 07f42c6..e7c87f7 100644
--- a/parted/parted.c
+++ b/parted/parted.c
@@ -524,18 +524,14 @@ error:
         return 0;
 }

-/* Strip whitespace character from end of the string, in place */
+/* Strip blanks from the end of string STR, in place.  */
 void _strip_trailing_spaces(char *str)
 {
         if (!str)
                 return;
-
-        size_t i;
-        for (i = strlen(str) - 1; i; i--)
-                if (c_isblank(str[i]))
-                    str[i]='\0';
-        else
-                break;
+        size_t i = strlen(str);
+        while (i && c_isblank(str[--i]))
+                str[i]='\0';
 }

 /* Return true, if str ends with [kMGT]iB, i.e. the IEC unit */
--
1.7.7.1.476.g9890


>From de7e662eceeb40214ec34be8d15a61f8a2508a36 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering at redhat.com>
Date: Thu, 27 Oct 2011 18:23:00 +0200
Subject: [PATCH 3/4] recognize all IEC units ([kMGTPEZY]iB), not just kMGT

* bootstrap.conf (gnulib_modules): Add these: c-ctype c-strcase
* parted/parted.c (_string_ends_with_iec_unit):
Recognize all IEC units ([kMGTPEZY]iB), not just kMGT.
Also, recognize "iB" without regard to case or locale.
---
 bootstrap.conf  |    2 ++
 parted/parted.c |   11 ++++-------
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/bootstrap.conf b/bootstrap.conf
index 7c79d50..14123a2 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -28,6 +28,8 @@ gnulib_modules="
   announce-gen
   argmatch
   assert
+  c-ctype
+  c-strcase
   calloc-gnu
   canonicalize-lgpl
   close
diff --git a/parted/parted.c b/parted/parted.c
index e7c87f7..f43ca49 100644
--- a/parted/parted.c
+++ b/parted/parted.c
@@ -54,6 +54,7 @@
 #include <unistd.h>
 #include <limits.h>
 #include "c-ctype.h"
+#include "c-strcase.h"
 #include "xalloc.h"

 #ifdef ENABLE_MTRACE
@@ -534,7 +535,7 @@ void _strip_trailing_spaces(char *str)
                 str[i]='\0';
 }

-/* Return true, if str ends with [kMGT]iB, i.e. the IEC unit */
+/* Return true, if str ends with [kMGTPEZY]iB, i.e. IEC units.  */
 static bool
 _string_ends_with_iec_unit(const char *str)
 {
@@ -542,12 +543,8 @@ _string_ends_with_iec_unit(const char *str)
         if (!str || strlen(str) < 4)
                 return false;

-        char const *last3 = str + strlen(str) - 3;
-        if (!strcmp(last3, "kiB") || !strcmp(last3, "MiB") ||
-            !strcmp(last3, "GiB") || !strcmp(last3, "TiB"))
-                return true;
-
-        return false;
+        char const *p = str + strlen(str) - 3;
+        return strchr ("kMGTPEZY", *p) && c_strcasecmp (p+1, "iB") == 0;
 }

 /* If the selected unit is one of kiB, MiB, GiB or TiB and the partition is not
--
1.7.7.1.476.g9890


>From 4a019eadda70ea2def7ca2f4936aca162c638d9a Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering at redhat.com>
Date: Fri, 28 Oct 2011 19:17:44 +0200
Subject: [PATCH 4/4] tests: add a test case for new IEC-specified-end feature

* tests/t0208-mkpart-end-in-IEC.sh (dev): Add a test to demonstrate
that demonstrates an off-by-one error.
Reformat to make each test occupy fewer lines.
---
 tests/t0208-mkpart-end-in-IEC.sh |   24 ++++++++++--------------
 1 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/tests/t0208-mkpart-end-in-IEC.sh b/tests/t0208-mkpart-end-in-IEC.sh
index cbd378d..23abd27 100644
--- a/tests/t0208-mkpart-end-in-IEC.sh
+++ b/tests/t0208-mkpart-end-in-IEC.sh
@@ -25,28 +25,23 @@ dev=dev-file

 dd if=/dev/null of=$dev bs=1M seek=$n_mbs || fail=1
 # create 1st partition
-parted --align=none -s $dev mklabel gpt mkpart p1 1MiB 2MiB \
-    > err 2>&1 || fail=1
-
-# expect no output
-compare err /dev/null || fail=1
+parted --align=none -s $dev mklabel gpt mkpart p1 1MiB 2MiB > err 2>&1 || fail=1
+compare err /dev/null || fail=1  # expect no output
 #parted -m -s $dev u s p > exp || fail=1

 # create 2nd partition - they should not overlap
 # this time specify default unit
-parted --align=none -s $dev unit MiB mkpart p2 2 3 \
-    > err 2>&1 || fail=1
-
-# expect no output
-compare err /dev/null || fail=1
+parted --align=none -s $dev unit MiB mkpart p2 2 3 > err 2>&1 || fail=1
+compare err /dev/null || fail=1  # expect no output

 # create 3rd partition - expect no overlap
 # specify default unit, but explicitly override it
-parted --align=none -s $dev unit TB mkpart p3 3MiB 4MiB \
-    > err 2>&1 || fail=1
+parted --align=none -s $dev unit TB mkpart p3 3MiB 4MiB > err 2>&1 || fail=1
+compare err /dev/null || fail=1  # expect no output

-# expect no output
-compare err /dev/null || fail=1
+# Specify default unit of MiB, yet use explicit ending sector number.
+parted --align=none -s $dev unit MiB mkpart p4 4MiB 10239s > err 2>&1 || fail=1
+compare err /dev/null || fail=1  # expect no output

 # check boundaries of the partitions
 parted -m -s $dev u s p > out || fail=1
@@ -56,6 +51,7 @@ cat <<EOF > exp || framework_failure
 1:2048s:4095s:2048s::p1:;
 2:4096s:6143s:2048s::p2:;
 3:6144s:8191s:2048s::p3:;
+4:8192s:10239s:2048s:p4:;
 EOF

 # compare expected and actual outputs
--
1.7.7.1.476.g9890



More information about the parted-devel mailing list