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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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
