Hello, attached two small patches: === Subject: [PATCH 1/2] numfmt: improve --field parsing, add tests
* src/numfmt.c: parse_field_arg() detect and fail on negative values (previously, implicit conversion from signed strtol to unsigned size_t failed to detect those); detect and fail on invalid ranges '1-2-3'; avoid adding superfluous range item for 'N-N' ranges. * tests/misc/numfmt.pl: test above scenarios, add few more tests for better coverage. Subject: [PATCH 2/2] numfmt: fix coding style * src/numfmt.c: parse_field_arg(): change whitespace and braces, no change in functionality. === regards, - assaf
>From ed5327af622ca0dbefe49f375954fa071396194e Mon Sep 17 00:00:00 2001 From: Assaf Gordon <[email protected]> Date: Fri, 10 Jul 2015 20:46:46 -0400 Subject: [PATCH 1/2] numfmt: improve --field parsing, add tests * src/numfmt.c: parse_field_arg() detect and fail on negative values (previously, implicit conversion from signed strtol to unsigned size_t failed to detect those); detect and fail on invalid ranges '1-2-3'; avoid adding superfluous range item for 'N-N' ranges. * tests/misc/numfmt.pl: test above scenarios, add few more tests for better coverage. --- src/numfmt.c | 29 +++++++++++++++++++++-------- tests/misc/numfmt.pl | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 8 deletions(-) diff --git a/src/numfmt.c b/src/numfmt.c index 35c5c5b..3e88045 100644 --- a/src/numfmt.c +++ b/src/numfmt.c @@ -1363,6 +1363,8 @@ parse_field_arg (char *arg) char *start, *end; range_pair_t *rp; + long l; + bool allow_range = true; size_t field_val; size_t range_val = 0; @@ -1380,12 +1382,13 @@ parse_field_arg (char *arg) /* range -M */ ++start; - all_fields_before = strtol (start, &end, 10); + l = strtol (start, &end, 10); - if (start == end || all_fields_before <=0) + if (start == end || l <=0) error (EXIT_FAILURE, 0, _("invalid field value %s"), quote (start)); + all_fields_before = l; return; } @@ -1393,11 +1396,12 @@ parse_field_arg (char *arg) NULL, NULL, free_field, false); while (*end != '\0') { - field_val = strtol (start, &end, 10); + l = strtol (start, &end, 10); - if (start == end || field_val <=0) + if (start == end || l <=0) error (EXIT_FAILURE, 0, _("invalid field value %s"), quote (start)); + field_val = l; if (! range_val) { @@ -1405,6 +1409,7 @@ parse_field_arg (char *arg) rp = xmalloc (sizeof (*rp)); rp->lo = rp->hi = field_val; gl_sortedlist_add (field_list, sort_field, rp); + allow_range = true; } else { @@ -1415,12 +1420,18 @@ parse_field_arg (char *arg) range end. */ if (field_val < range_val) error (EXIT_FAILURE, 0, _("invalid decreasing range")); - rp = xmalloc (sizeof (*rp)); - rp->lo = range_val + 1; - rp->hi = field_val; - gl_sortedlist_add (field_list, sort_field, rp); + + /* skip ranges 'N-N', value N was added in the previous iteration */ + if (field_val != range_val) + { + rp = xmalloc (sizeof (*rp)); + rp->lo = range_val + 1; + rp->hi = field_val; + gl_sortedlist_add (field_list, sort_field, rp); + } range_val = 0; + allow_range = false; } switch (*end) { @@ -1432,6 +1443,8 @@ parse_field_arg (char *arg) case '-': /* field range separator */ + if (!allow_range) + error (EXIT_FAILURE, 0, _("invalid field range")); ++end; start = end; range_val = field_val; diff --git a/tests/misc/numfmt.pl b/tests/misc/numfmt.pl index 0e4dc79..5d18842 100755 --- a/tests/misc/numfmt.pl +++ b/tests/misc/numfmt.pl @@ -197,6 +197,8 @@ my @Tests = ['delim-4', '--delimiter=: --from=auto 40M:60M', {OUT=>'40000000:60M'}], ['delim-5', '-d: --field=2 --from=auto :40M:60M', {OUT=>':40000000:60M'}], ['delim-6', '-d: --field 3 --from=auto 40M:60M', {OUT=>"40M:60M"}], + ['delim-err-1', '-d,, --to=si 1', {EXIT=>1}, + {ERR => "$prog: the delimiter must be a single character\n"}], #Fields ['field-1', '--field A', @@ -244,9 +246,45 @@ my @Tests = ['field-range-7', '--field -3 --to=si "1000 2000 3000 4000 5000"', {OUT=>"1.0K 2.0K 3.0K 4000 5000"}], + ['field-range-8', '--field 1-2,4-5 --to=si "1000 2000 3000 4000 5000"', + {OUT=>"1.0K 2.0K 3000 4.0K 5.0K"}], + ['field-range-9', '--field 4-5,1-2 --to=si "1000 2000 3000 4000 5000"', + {OUT=>"1.0K 2.0K 3000 4.0K 5.0K"}], + + ['field-range-10','--field 1-3,2-4 --to=si "1000 2000 3000 4000 5000"', + {OUT=>"1.0K 2.0K 3.0K 4.0K 5000"}], + ['field-range-11','--field 2-4,1-3 --to=si "1000 2000 3000 4000 5000"', + {OUT=>"1.0K 2.0K 3.0K 4.0K 5000"}], + + ['field-range-12','--field 1-1,3-3 --to=si "1000 2000 3000 4000 5000"', + {OUT=>"1.0K 2000 3.0K 4000 5000"}], + ['all-fields-1', '--field=- --to=si "1000 2000 3000 4000 5000"', {OUT=>"1.0K 2.0K 3.0K 4.0K 5.0K"}], + ['field-range-err-1', '--field -foo --to=si 10', + {EXIT=>1}, {ERR=>"$prog: invalid field value 'foo'\n"}], + ['field-range-err-2', '--field --3 --to=si 10', + {EXIT=>1}, {ERR=>"$prog: invalid field value '-3'\n"}], + ['field-range-err-3', '--field 0 --to=si 10', + {EXIT=>1}, {ERR=>"$prog: invalid field value '0'\n"}], + ['field-range-err-4', '--field 3-2 --to=si 10', + {EXIT=>1}, {ERR=>"$prog: invalid decreasing range\n"}], + ['field-range-err-5', '--field 1,-2 --to=si 10', + {EXIT=>1}, {ERR=>"$prog: invalid field value '-2'\n"}], + ['field-range-err-6', '--field - --field 1- --to=si 10', + {EXIT=>1}, {ERR=>"$prog: multiple field specifications\n"}], + ['field-range-err-7', '--field -1 --field 1- --to=si 10', + {EXIT=>1}, {ERR=>"$prog: multiple field specifications\n"}], + ['field-range-err-8', '--field -1 --field 1,2,3 --to=si 10', + {EXIT=>1}, {ERR=>"$prog: multiple field specifications\n"}], + ['field-range-err-9', '--field 1- --field 1,2,3 --to=si 10', + {EXIT=>1}, {ERR=>"$prog: multiple field specifications\n"}], + ['field-range-err-10','--field 1,2,3 --field 1- --to=si 10', + {EXIT=>1}, {ERR=>"$prog: multiple field specifications\n"}], + ['field-range-err-11','--field 1-2-3 --field 1- --to=si 10', + {EXIT=>1}, {ERR=>"$prog: invalid field range\n"}], + # Auto-consume white-space, setup auto-padding ['whitespace-1', '--to=si --field 2 "A 500 B"', {OUT=>"A 500 B"}], ['whitespace-2', '--to=si --field 2 "A 5000 B"', {OUT=>"A 5.0K B"}], -- 1.9.1 >From c324deff7a064a4588b4a847b4b316e610da30ed Mon Sep 17 00:00:00 2001 From: Assaf Gordon <[email protected]> Date: Fri, 10 Jul 2015 20:53:29 -0400 Subject: [PATCH 2/2] numfmt: fix coding style * src/numfmt.c: parse_field_arg(): change whitespace and braces, no change in functionality. --- src/numfmt.c | 104 ++++++++++++++++++++++++++++++----------------------------- 1 file changed, 53 insertions(+), 51 deletions(-) diff --git a/src/numfmt.c b/src/numfmt.c index 3e88045..feade6c 100644 --- a/src/numfmt.c +++ b/src/numfmt.c @@ -1395,62 +1395,64 @@ parse_field_arg (char *arg) field_list = gl_list_create_empty (GL_LINKED_LIST, NULL, NULL, free_field, false); - while (*end != '\0') { - l = strtol (start, &end, 10); + while (*end != '\0') + { + l = strtol (start, &end, 10); - if (start == end || l <=0) - error (EXIT_FAILURE, 0, _("invalid field value %s"), - quote (start)); - field_val = l; + if (start == end || l <=0) + error (EXIT_FAILURE, 0, _("invalid field value %s"), + quote (start)); + field_val = l; - if (! range_val) - { - /* field N */ - rp = xmalloc (sizeof (*rp)); - rp->lo = rp->hi = field_val; - gl_sortedlist_add (field_list, sort_field, rp); - allow_range = true; - } - else - { - /* range N-M - The last field was the start of the field range. The current - field is the end of the field range. We already added the - start field, so increment and add all the fields through - range end. */ - if (field_val < range_val) - error (EXIT_FAILURE, 0, _("invalid decreasing range")); - - /* skip ranges 'N-N', value N was added in the previous iteration */ - if (field_val != range_val) - { - rp = xmalloc (sizeof (*rp)); - rp->lo = range_val + 1; - rp->hi = field_val; - gl_sortedlist_add (field_list, sort_field, rp); - } - - range_val = 0; - allow_range = false; - } + if (! range_val) + { + /* field N */ + rp = xmalloc (sizeof (*rp)); + rp->lo = rp->hi = field_val; + gl_sortedlist_add (field_list, sort_field, rp); + allow_range = true; + } + else + { + /* range N-M + The last field was the start of the field range. The current + field is the end of the field range. We already added the + start field, so increment and add all the fields through + range end. */ + if (field_val < range_val) + error (EXIT_FAILURE, 0, _("invalid decreasing range")); + + /* skip ranges 'N-N', value N was added in the previous iteration */ + if (field_val != range_val) + { + rp = xmalloc (sizeof (*rp)); + rp->lo = range_val + 1; + rp->hi = field_val; + gl_sortedlist_add (field_list, sort_field, rp); + } - switch (*end) { - case ',': - /* discrete field separator */ - ++end; - start = end; - break; + range_val = 0; + allow_range = false; + } - case '-': - /* field range separator */ - if (!allow_range) - error (EXIT_FAILURE, 0, _("invalid field range")); - ++end; - start = end; - range_val = field_val; - break; + switch (*end) + { + case ',': + /* discrete field separator */ + ++end; + start = end; + break; + + case '-': + /* field range separator */ + if (!allow_range) + error (EXIT_FAILURE, 0, _("invalid field range")); + ++end; + start = end; + range_val = field_val; + break; + } } - } if (range_val) { -- 1.9.1
