On 10/24/2012 10:26 AM, Pádraig Brady wrote:
On 10/24/2012 09:03 AM, Ondrej Oprala wrote:
Hello,
one of our customers made a bug report, stating that pr -nNUM FILE
causes a floating point exception if NUM >= 32
and I would really like to hear your opinion on how to solve this.
The easiest solution would be to document the <32 limit for the -n option.
The more difficult one would be to support numbers of arbitrary length.
I was thinking about the second solution and AFAIK the line number is not
used in any arithmetic (apart from incrementation), so it could be represented
as
an array of digits, with a function incrementing the array from the rightmost
element
as needed. Then we would just print out the array one element at a time. It
would
probably cause a performance penalty though.
Thanks in advance for any advice or suggestions.
Ouch. Confirmed that this dumps core:
echo . | pr -T -n.32
We can fix that up with something like the following.
I'll add a test and commit.
Complete proposed patch is attached.
thanks,
Pádraig.
>From 9dc0bffd25a4d202d8beb875290901834c33d873 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email protected]>
Date: Wed, 24 Oct 2012 10:55:13 +0100
Subject: [PATCH] pr: fix -n to pad consistently and not crash
* src/pr.c: Replace the code to truncate the most significant
digits of line numbers, with much simpler string manipulation
that supports arbitrary widths. Before this, specifying a
width >= 32 to -n would result in a divide by zero error.
Also remove the inconsistent padding with zeros and spaces, which
would result in zero padding for widths 12 and 15.
* tests/pr/pr-tests.pl: Added a test to ensure no zero padding,
and also a test for the divide by zero case.
* NEWS: Mentioned the fix
Reported by Ondrej Oprala
---
NEWS | 6 ++++++
src/pr.c | 28 ++++++----------------------
tests/pr/pr-tests.pl | 7 +++++++
3 files changed, 19 insertions(+), 22 deletions(-)
diff --git a/NEWS b/NEWS
index 04721da..e13ba22 100644
--- a/NEWS
+++ b/NEWS
@@ -2,6 +2,12 @@ GNU coreutils NEWS -*- outline -*-
* Noteworthy changes in release ?.? (????-??-??) [?]
+** Bug fixes
+
+ pr -n no longer crashes when passed values >= 32. Also line numbers are
+ consistently padded with spaces, rather than with zeros for certain widths.
+ [bug introduced in TEXTUTILS-1_22i]
+
* Noteworthy changes in release 8.20 (2012-10-23) [stable]
diff --git a/src/pr.c b/src/pr.c
index e97c434..3e40167 100644
--- a/src/pr.c
+++ b/src/pr.c
@@ -630,10 +630,6 @@ static uintmax_t page_number;
2 moo 4 hoo 6 zoo */
static int line_number;
-/* With line_number overflow, we use power_10 to cut off the higher-order
- digits of the line_number */
-static int power_10;
-
/* (-n) True means lines should be preceded by numbers. */
static bool numbered_lines = false;
@@ -1268,7 +1264,6 @@ init_parameters (int number_of_files)
if (numbered_lines)
{
- int tmp_i;
int chars_per_default_tab = 8;
line_count = start_line_num;
@@ -1289,12 +1284,6 @@ init_parameters (int number_of_files)
printing files in parallel. */
if (parallel_files)
chars_used_by_number = number_width;
-
- /* We use power_10 to cut off the higher-order digits of the
- line_number in function add_line_number */
- tmp_i = chars_per_number;
- for (power_10 = 1; tmp_i > 0; --tmp_i)
- power_10 = 10 * power_10;
}
chars_per_column = (chars_per_line - chars_used_by_number
@@ -1306,7 +1295,8 @@ init_parameters (int number_of_files)
if (numbered_lines)
{
free (number_buff);
- number_buff = xmalloc (2 * chars_per_number);
+ number_buff = xmalloc (MAX (chars_per_number,
+ INT_STRLEN_BOUND (line_number)) + 1);
}
/* Pick the maximum between the tab width and the width of an
@@ -2029,19 +2019,13 @@ add_line_number (COLUMN *p)
{
int i;
char *s;
- int left_cut;
+ int num_width;
/* Cutting off the higher-order digits is more informative than
- lower-order cut off*/
- if (line_number < power_10)
- sprintf (number_buff, "%*d", chars_per_number, line_number);
- else
- {
- left_cut = line_number % power_10;
- sprintf (number_buff, "%0*d", chars_per_number, left_cut);
- }
+ lower-order cut off. */
+ num_width = sprintf (number_buff, "%*d", chars_per_number, line_number);
line_number++;
- s = number_buff;
+ s = number_buff + (num_width - chars_per_number);
for (i = chars_per_number; i > 0; i--)
(p->char_func) (*s++);
diff --git a/tests/pr/pr-tests.pl b/tests/pr/pr-tests.pl
index f202414..aeaa7fe 100755
--- a/tests/pr/pr-tests.pl
+++ b/tests/pr/pr-tests.pl
@@ -407,6 +407,13 @@ my @tv = (
# Before coreutils-8.13 page numbers were not handled correctly when
# headers were not printed (when -l <= 10 or -t or -T specified)
['page-range', '+1:1 -2 -l1 -s" "', "a\nb\nc\n", "a b\n", 0],
+
+# This padded with zeros before coreutils-8.21
+['padding1', '-t -n,15', "1\n", (" "x 14)."1,1\n", 0],
+# This crashed with divide by zero before coreutils-8.21
+['padding2', '-t -n,64', "1\n", (" "x 63)."1,1\n", 0],
+# Ensure we handle buffer truncation correctly
+['padding3', '-t -N1000000 -n,1', "1\n", "0,1\n", 0],
);
# Convert the above old-style test vectors to the newer
--
1.7.6.4
