On 12/06/2012 02:11 AM, Cojocaru Alexandru wrote:
From 82f2b062c0e21d9a0d64f9ceab363d2a79f5a6eb Mon Sep 17 00:00:00 2001
From: Cojocaru Alexandru <xo...@gmx.com>
Date: Thu, 6 Dec 2012 03:03:41 +0100
Subject: [PATCH] cut: fix memory leak
* src/cut.c (set_fields): don't allocate memory for
`printable_field' if there are no finite ranges.
The bug was introduced on 2012-2-7 via commit 2e636af.
---
src/cut.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/src/cut.c b/src/cut.c
index 4219d24..87abd15 100644
--- a/src/cut.c
+++ b/src/cut.c
@@ -500,14 +500,13 @@ set_fields (const char *fieldstr)
if (rp[i].hi > max_range_endpoint)
max_range_endpoint = rp[i].hi;
}
- if (max_range_endpoint < eol_range_start)
- max_range_endpoint = eol_range_start;
/* Allocate an array large enough so that it may be indexed by
the field numbers corresponding to all finite ranges
(i.e. '2-6' or '-4', but not '5-') in FIELDSTR. */
- printable_field = xzalloc (max_range_endpoint / CHAR_BIT + 1);
+ if (n_rp)
+ printable_field = xzalloc (max_range_endpoint / CHAR_BIT + 1);
qsort (rp, n_rp, sizeof (rp[0]), compare_ranges);
@@ -531,8 +530,11 @@ set_fields (const char *fieldstr)
if (output_delimiter_specified
&& !complement
- && eol_range_start && !is_printable_field (eol_range_start))
- mark_range_start (eol_range_start);
+ && eol_range_start
+ && printable_field && !is_printable_field (eol_range_start))
+ {
+ mark_range_start (eol_range_start);
+ }
free (rp);
This looks right. subsequent accesses to the now not alloced bit array,
are avoided when max_range_endpoint = 0.
$ valgrind cut-before --output=" " -f1234567- /dev/null 2>&1 | grep reachable
==20949== still reachable: 154,323 bytes in 2 blocks
$ valgrind cut-after --output=" " -f1234567- /dev/null 2>&1 | grep reachable
==20816== still reachable: 2 bytes in 1 blocks
I wouldn't describe it as a leak though, rather a redundant allocation.
Since this is an edge case minor performance issue, I don't think
it needs a NEWS entry and will just adjust the commit summary a little
to say "advoid a redundant memory allocation".
Hmm, it might be a bit more consistent to guard all
references to the bit vector array with max_range_endpoint?
How about the attached?
thanks!
Pádraig.
>From c40543afe9fd5a06fac60f47ae92775303f83b49 Mon Sep 17 00:00:00 2001
From: Cojocaru Alexandru <xo...@gmx.com>
Date: Thu, 6 Dec 2012 03:03:41 +0100
Subject: [PATCH] cut: avoid a redundant heap allocation
* src/cut.c (set_fields): Don't allocate memory for
`printable_field' if there are no finite ranges.
The extra allocation was introduced via commit v8.10-3-g2e636af.
---
src/cut.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/cut.c b/src/cut.c
index dc83072..de9320c 100644
--- a/src/cut.c
+++ b/src/cut.c
@@ -500,14 +500,13 @@ set_fields (const char *fieldstr)
if (rp[i].hi > max_range_endpoint)
max_range_endpoint = rp[i].hi;
}
- if (max_range_endpoint < eol_range_start)
- max_range_endpoint = eol_range_start;
/* Allocate an array large enough so that it may be indexed by
the field numbers corresponding to all finite ranges
(i.e. '2-6' or '-4', but not '5-') in FIELDSTR. */
- printable_field = xzalloc (max_range_endpoint / CHAR_BIT + 1);
+ if (max_range_endpoint)
+ printable_field = xzalloc (max_range_endpoint / CHAR_BIT + 1);
qsort (rp, n_rp, sizeof (rp[0]), compare_ranges);
@@ -531,7 +530,8 @@ set_fields (const char *fieldstr)
if (output_delimiter_specified
&& !complement
- && eol_range_start && !is_printable_field (eol_range_start))
+ && eol_range_start
+ && max_range_endpoint && !is_printable_field (eol_range_start))
mark_range_start (eol_range_start);
free (rp);
--
1.7.6.4