* [PATCH ethtool] ethtool.c: fix memory leaks
@ 2016-03-18 12:24 Ivan Vecera
2016-04-08 8:06 ` Ivan Vecera
2016-06-26 8:59 ` Ben Hutchings
0 siblings, 2 replies; 5+ messages in thread
From: Ivan Vecera @ 2016-03-18 12:24 UTC (permalink / raw)
To: netdev; +Cc: bwh
Memory allocated at several places is not appropriately freed.
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
ethtool.c | 60 +++++++++++++++++++++++++++++++++++++++++++++---------------
1 file changed, 45 insertions(+), 15 deletions(-)
diff --git a/ethtool.c b/ethtool.c
index 0cd0d4f..ca0bf28 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -2065,10 +2065,14 @@ static int do_gfeatures(struct cmd_context *ctx)
features = get_features(ctx, defs);
if (!features) {
fprintf(stdout, "no feature info available\n");
+ free(defs);
return 1;
}
dump_features(defs, features, NULL);
+
+ free(features);
+ free(defs);
return 0;
}
@@ -2078,11 +2082,11 @@ static int do_sfeatures(struct cmd_context *ctx)
int any_changed = 0, any_mismatch = 0;
u32 off_flags_wanted = 0;
u32 off_flags_mask = 0;
- struct ethtool_sfeatures *efeatures;
+ struct ethtool_sfeatures *efeatures = NULL;
struct cmdline_info *cmdline_features;
- struct feature_state *old_state, *new_state;
+ struct feature_state *old_state = NULL, *new_state = NULL;
struct ethtool_value eval;
- int err;
+ int err, retval = 1;
int i, j;
defs = get_feature_defs(ctx);
@@ -2096,7 +2100,7 @@ static int do_sfeatures(struct cmd_context *ctx)
sizeof(efeatures->features[0]));
if (!efeatures) {
perror("Cannot parse arguments");
- return 1;
+ goto finish;
}
efeatures->cmd = ETHTOOL_SFEATURES;
efeatures->size = FEATURE_BITS_TO_BLOCKS(defs->n_features);
@@ -2114,7 +2118,7 @@ static int do_sfeatures(struct cmd_context *ctx)
sizeof(cmdline_features[0]));
if (!cmdline_features) {
perror("Cannot parse arguments");
- return 1;
+ goto finish;
}
for (i = 0; i < ARRAY_SIZE(off_flag_def); i++)
flag_to_cmdline_info(off_flag_def[i].short_name,
@@ -2133,12 +2137,13 @@ static int do_sfeatures(struct cmd_context *ctx)
if (!any_changed) {
fprintf(stdout, "no features changed\n");
- return 0;
+ retval = 0;
+ goto finish;
}
old_state = get_features(ctx, defs);
if (!old_state)
- return 1;
+ goto finish;
if (efeatures) {
/* For each offload that the user specified, update any
@@ -2182,7 +2187,7 @@ static int do_sfeatures(struct cmd_context *ctx)
err = send_ioctl(ctx, efeatures);
if (err < 0) {
perror("Cannot set device feature settings");
- return 1;
+ goto finish;
}
} else {
for (i = 0; i < ARRAY_SIZE(off_flag_def); i++) {
@@ -2197,7 +2202,7 @@ static int do_sfeatures(struct cmd_context *ctx)
fprintf(stderr,
"Cannot set device %s settings: %m\n",
off_flag_def[i].long_name);
- return 1;
+ goto finish;
}
}
}
@@ -2211,7 +2216,8 @@ static int do_sfeatures(struct cmd_context *ctx)
err = send_ioctl(ctx, &eval);
if (err) {
perror("Cannot set device flag settings");
- return 92;
+ retval = 92;
+ goto finish;
}
}
}
@@ -2219,7 +2225,7 @@ static int do_sfeatures(struct cmd_context *ctx)
/* Compare new state with requested state */
new_state = get_features(ctx, defs);
if (!new_state)
- return 1;
+ goto finish;
any_changed = new_state->off_flags != old_state->off_flags;
any_mismatch = (new_state->off_flags !=
((old_state->off_flags & ~off_flags_mask) |
@@ -2238,13 +2244,19 @@ static int do_sfeatures(struct cmd_context *ctx)
if (!any_changed) {
fprintf(stderr,
"Could not change any device features\n");
- return 1;
+ goto finish;
}
printf("Actual changes:\n");
dump_features(defs, new_state, old_state);
}
- return 0;
+ retval = 0;
+finish:
+ free(new_state);
+ free(old_state);
+ free(efeatures);
+ free(defs);
+ return retval;
}
static int do_gset(struct cmd_context *ctx)
@@ -2705,8 +2717,18 @@ static int do_gregs(struct cmd_context *ctx)
return 75;
}
- regs = realloc(regs, sizeof(*regs) + st.st_size);
- regs->len = st.st_size;
+ if (regs->len != st.st_size) {
+ struct ethtool_regs *new_regs;
+ new_regs = realloc(regs, sizeof(*regs) + st.st_size);
+ if (!new_regs) {
+ perror("Cannot allocate memory for register "
+ "dump");
+ free(regs);
+ return 73;
+ }
+ regs = new_regs;
+ regs->len = st.st_size;
+ }
nread = fread(regs->data, regs->len, 1, f);
fclose(f);
if (nread != 1) {
@@ -3775,6 +3797,7 @@ static int do_gprivflags(struct cmd_context *ctx)
}
if (strings->len == 0) {
fprintf(stderr, "No private flags defined\n");
+ free(strings);
return 1;
}
if (strings->len > 32) {
@@ -3786,6 +3809,7 @@ static int do_gprivflags(struct cmd_context *ctx)
flags.cmd = ETHTOOL_GPFLAGS;
if (send_ioctl(ctx, &flags)) {
perror("Cannot get private flags");
+ free(strings);
return 1;
}
@@ -3804,6 +3828,7 @@ static int do_gprivflags(struct cmd_context *ctx)
(const char *)strings->data + i * ETH_GSTRING_LEN,
(flags.data & (1U << i)) ? "on" : "off");
+ free(strings);
return 0;
}
@@ -3825,6 +3850,7 @@ static int do_sprivflags(struct cmd_context *ctx)
}
if (strings->len == 0) {
fprintf(stderr, "No private flags defined\n");
+ free(strings);
return 1;
}
if (strings->len > 32) {
@@ -3836,6 +3862,7 @@ static int do_sprivflags(struct cmd_context *ctx)
cmdline = calloc(strings->len, sizeof(*cmdline));
if (!cmdline) {
perror("Cannot parse arguments");
+ free(strings);
return 1;
}
for (i = 0; i < strings->len; i++) {
@@ -3852,6 +3879,7 @@ static int do_sprivflags(struct cmd_context *ctx)
flags.cmd = ETHTOOL_GPFLAGS;
if (send_ioctl(ctx, &flags)) {
perror("Cannot get private flags");
+ free(strings);
return 1;
}
@@ -3859,9 +3887,11 @@ static int do_sprivflags(struct cmd_context *ctx)
flags.data = (flags.data & ~seen_flags) | wanted_flags;
if (send_ioctl(ctx, &flags)) {
perror("Cannot set private flags");
+ free(strings);
return 1;
}
+ free(strings);
return 0;
}
--
2.7.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH ethtool] ethtool.c: fix memory leaks
2016-03-18 12:24 [PATCH ethtool] ethtool.c: fix memory leaks Ivan Vecera
@ 2016-04-08 8:06 ` Ivan Vecera
2016-06-26 8:59 ` Ben Hutchings
1 sibling, 0 replies; 5+ messages in thread
From: Ivan Vecera @ 2016-04-08 8:06 UTC (permalink / raw)
To: netdev; +Cc: bwh
On 18.3.2016 13:24, Ivan Vecera wrote:
> Memory allocated at several places is not appropriately freed.
>
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
Ben, ping.
I.
> ---
> ethtool.c | 60 +++++++++++++++++++++++++++++++++++++++++++++---------------
> 1 file changed, 45 insertions(+), 15 deletions(-)
>
> diff --git a/ethtool.c b/ethtool.c
> index 0cd0d4f..ca0bf28 100644
> --- a/ethtool.c
> +++ b/ethtool.c
> @@ -2065,10 +2065,14 @@ static int do_gfeatures(struct cmd_context *ctx)
> features = get_features(ctx, defs);
> if (!features) {
> fprintf(stdout, "no feature info available\n");
> + free(defs);
> return 1;
> }
>
> dump_features(defs, features, NULL);
> +
> + free(features);
> + free(defs);
> return 0;
> }
>
> @@ -2078,11 +2082,11 @@ static int do_sfeatures(struct cmd_context *ctx)
> int any_changed = 0, any_mismatch = 0;
> u32 off_flags_wanted = 0;
> u32 off_flags_mask = 0;
> - struct ethtool_sfeatures *efeatures;
> + struct ethtool_sfeatures *efeatures = NULL;
> struct cmdline_info *cmdline_features;
> - struct feature_state *old_state, *new_state;
> + struct feature_state *old_state = NULL, *new_state = NULL;
> struct ethtool_value eval;
> - int err;
> + int err, retval = 1;
> int i, j;
>
> defs = get_feature_defs(ctx);
> @@ -2096,7 +2100,7 @@ static int do_sfeatures(struct cmd_context *ctx)
> sizeof(efeatures->features[0]));
> if (!efeatures) {
> perror("Cannot parse arguments");
> - return 1;
> + goto finish;
> }
> efeatures->cmd = ETHTOOL_SFEATURES;
> efeatures->size = FEATURE_BITS_TO_BLOCKS(defs->n_features);
> @@ -2114,7 +2118,7 @@ static int do_sfeatures(struct cmd_context *ctx)
> sizeof(cmdline_features[0]));
> if (!cmdline_features) {
> perror("Cannot parse arguments");
> - return 1;
> + goto finish;
> }
> for (i = 0; i < ARRAY_SIZE(off_flag_def); i++)
> flag_to_cmdline_info(off_flag_def[i].short_name,
> @@ -2133,12 +2137,13 @@ static int do_sfeatures(struct cmd_context *ctx)
>
> if (!any_changed) {
> fprintf(stdout, "no features changed\n");
> - return 0;
> + retval = 0;
> + goto finish;
> }
>
> old_state = get_features(ctx, defs);
> if (!old_state)
> - return 1;
> + goto finish;
>
> if (efeatures) {
> /* For each offload that the user specified, update any
> @@ -2182,7 +2187,7 @@ static int do_sfeatures(struct cmd_context *ctx)
> err = send_ioctl(ctx, efeatures);
> if (err < 0) {
> perror("Cannot set device feature settings");
> - return 1;
> + goto finish;
> }
> } else {
> for (i = 0; i < ARRAY_SIZE(off_flag_def); i++) {
> @@ -2197,7 +2202,7 @@ static int do_sfeatures(struct cmd_context *ctx)
> fprintf(stderr,
> "Cannot set device %s settings: %m\n",
> off_flag_def[i].long_name);
> - return 1;
> + goto finish;
> }
> }
> }
> @@ -2211,7 +2216,8 @@ static int do_sfeatures(struct cmd_context *ctx)
> err = send_ioctl(ctx, &eval);
> if (err) {
> perror("Cannot set device flag settings");
> - return 92;
> + retval = 92;
> + goto finish;
> }
> }
> }
> @@ -2219,7 +2225,7 @@ static int do_sfeatures(struct cmd_context *ctx)
> /* Compare new state with requested state */
> new_state = get_features(ctx, defs);
> if (!new_state)
> - return 1;
> + goto finish;
> any_changed = new_state->off_flags != old_state->off_flags;
> any_mismatch = (new_state->off_flags !=
> ((old_state->off_flags & ~off_flags_mask) |
> @@ -2238,13 +2244,19 @@ static int do_sfeatures(struct cmd_context *ctx)
> if (!any_changed) {
> fprintf(stderr,
> "Could not change any device features\n");
> - return 1;
> + goto finish;
> }
> printf("Actual changes:\n");
> dump_features(defs, new_state, old_state);
> }
>
> - return 0;
> + retval = 0;
> +finish:
> + free(new_state);
> + free(old_state);
> + free(efeatures);
> + free(defs);
> + return retval;
> }
>
> static int do_gset(struct cmd_context *ctx)
> @@ -2705,8 +2717,18 @@ static int do_gregs(struct cmd_context *ctx)
> return 75;
> }
>
> - regs = realloc(regs, sizeof(*regs) + st.st_size);
> - regs->len = st.st_size;
> + if (regs->len != st.st_size) {
> + struct ethtool_regs *new_regs;
> + new_regs = realloc(regs, sizeof(*regs) + st.st_size);
> + if (!new_regs) {
> + perror("Cannot allocate memory for register "
> + "dump");
> + free(regs);
> + return 73;
> + }
> + regs = new_regs;
> + regs->len = st.st_size;
> + }
> nread = fread(regs->data, regs->len, 1, f);
> fclose(f);
> if (nread != 1) {
> @@ -3775,6 +3797,7 @@ static int do_gprivflags(struct cmd_context *ctx)
> }
> if (strings->len == 0) {
> fprintf(stderr, "No private flags defined\n");
> + free(strings);
> return 1;
> }
> if (strings->len > 32) {
> @@ -3786,6 +3809,7 @@ static int do_gprivflags(struct cmd_context *ctx)
> flags.cmd = ETHTOOL_GPFLAGS;
> if (send_ioctl(ctx, &flags)) {
> perror("Cannot get private flags");
> + free(strings);
> return 1;
> }
>
> @@ -3804,6 +3828,7 @@ static int do_gprivflags(struct cmd_context *ctx)
> (const char *)strings->data + i * ETH_GSTRING_LEN,
> (flags.data & (1U << i)) ? "on" : "off");
>
> + free(strings);
> return 0;
> }
>
> @@ -3825,6 +3850,7 @@ static int do_sprivflags(struct cmd_context *ctx)
> }
> if (strings->len == 0) {
> fprintf(stderr, "No private flags defined\n");
> + free(strings);
> return 1;
> }
> if (strings->len > 32) {
> @@ -3836,6 +3862,7 @@ static int do_sprivflags(struct cmd_context *ctx)
> cmdline = calloc(strings->len, sizeof(*cmdline));
> if (!cmdline) {
> perror("Cannot parse arguments");
> + free(strings);
> return 1;
> }
> for (i = 0; i < strings->len; i++) {
> @@ -3852,6 +3879,7 @@ static int do_sprivflags(struct cmd_context *ctx)
> flags.cmd = ETHTOOL_GPFLAGS;
> if (send_ioctl(ctx, &flags)) {
> perror("Cannot get private flags");
> + free(strings);
> return 1;
> }
>
> @@ -3859,9 +3887,11 @@ static int do_sprivflags(struct cmd_context *ctx)
> flags.data = (flags.data & ~seen_flags) | wanted_flags;
> if (send_ioctl(ctx, &flags)) {
> perror("Cannot set private flags");
> + free(strings);
> return 1;
> }
>
> + free(strings);
> return 0;
> }
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH ethtool] ethtool.c: fix memory leaks
2016-03-18 12:24 [PATCH ethtool] ethtool.c: fix memory leaks Ivan Vecera
2016-04-08 8:06 ` Ivan Vecera
@ 2016-06-26 8:59 ` Ben Hutchings
2016-06-27 9:41 ` Ivan Vecera
1 sibling, 1 reply; 5+ messages in thread
From: Ben Hutchings @ 2016-06-26 8:59 UTC (permalink / raw)
To: Ivan Vecera, netdev; +Cc: bwh
[-- Attachment #1: Type: text/plain, Size: 6974 bytes --]
On Fri, 2016-03-18 at 13:24 +0100, Ivan Vecera wrote:
> Memory allocated at several places is not appropriately freed.
Given that ethtool is not a library or a long-running application - why
does that matter?
Ben.
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> ---
> ethtool.c | 60 +++++++++++++++++++++++++++++++++++++++++++++------
> ---------
> 1 file changed, 45 insertions(+), 15 deletions(-)
>
> diff --git a/ethtool.c b/ethtool.c
> index 0cd0d4f..ca0bf28 100644
> --- a/ethtool.c
> +++ b/ethtool.c
> @@ -2065,10 +2065,14 @@ static int do_gfeatures(struct cmd_context
> *ctx)
> features = get_features(ctx, defs);
> if (!features) {
> fprintf(stdout, "no feature info available\n");
> + free(defs);
> return 1;
> }
>
> dump_features(defs, features, NULL);
> +
> + free(features);
> + free(defs);
> return 0;
> }
>
> @@ -2078,11 +2082,11 @@ static int do_sfeatures(struct cmd_context
> *ctx)
> int any_changed = 0, any_mismatch = 0;
> u32 off_flags_wanted = 0;
> u32 off_flags_mask = 0;
> - struct ethtool_sfeatures *efeatures;
> + struct ethtool_sfeatures *efeatures = NULL;
> struct cmdline_info *cmdline_features;
> - struct feature_state *old_state, *new_state;
> + struct feature_state *old_state = NULL, *new_state = NULL;
> struct ethtool_value eval;
> - int err;
> + int err, retval = 1;
> int i, j;
>
> defs = get_feature_defs(ctx);
> @@ -2096,7 +2100,7 @@ static int do_sfeatures(struct cmd_context
> *ctx)
> sizeof(efeatures->features[0]));
> if (!efeatures) {
> perror("Cannot parse arguments");
> - return 1;
> + goto finish;
> }
> efeatures->cmd = ETHTOOL_SFEATURES;
> efeatures->size = FEATURE_BITS_TO_BLOCKS(defs-
> >n_features);
> @@ -2114,7 +2118,7 @@ static int do_sfeatures(struct cmd_context
> *ctx)
> sizeof(cmdline_features[0]));
> if (!cmdline_features) {
> perror("Cannot parse arguments");
> - return 1;
> + goto finish;
> }
> for (i = 0; i < ARRAY_SIZE(off_flag_def); i++)
> flag_to_cmdline_info(off_flag_def[i].short_name,
> @@ -2133,12 +2137,13 @@ static int do_sfeatures(struct cmd_context
> *ctx)
>
> if (!any_changed) {
> fprintf(stdout, "no features changed\n");
> - return 0;
> + retval = 0;
> + goto finish;
> }
>
> old_state = get_features(ctx, defs);
> if (!old_state)
> - return 1;
> + goto finish;
>
> if (efeatures) {
> /* For each offload that the user specified, update
> any
> @@ -2182,7 +2187,7 @@ static int do_sfeatures(struct cmd_context
> *ctx)
> err = send_ioctl(ctx, efeatures);
> if (err < 0) {
> perror("Cannot set device feature
> settings");
> - return 1;
> + goto finish;
> }
> } else {
> for (i = 0; i < ARRAY_SIZE(off_flag_def); i++) {
> @@ -2197,7 +2202,7 @@ static int do_sfeatures(struct cmd_context
> *ctx)
> fprintf(stderr,
> "Cannot set device
> %s settings: %m\n",
> off_flag_def[i].long
> _name);
> - return 1;
> + goto finish;
> }
> }
> }
> @@ -2211,7 +2216,8 @@ static int do_sfeatures(struct cmd_context
> *ctx)
> err = send_ioctl(ctx, &eval);
> if (err) {
> perror("Cannot set device flag
> settings");
> - return 92;
> + retval = 92;
> + goto finish;
> }
> }
> }
> @@ -2219,7 +2225,7 @@ static int do_sfeatures(struct cmd_context
> *ctx)
> /* Compare new state with requested state */
> new_state = get_features(ctx, defs);
> if (!new_state)
> - return 1;
> + goto finish;
> any_changed = new_state->off_flags != old_state->off_flags;
> any_mismatch = (new_state->off_flags !=
> ((old_state->off_flags & ~off_flags_mask) |
> @@ -2238,13 +2244,19 @@ static int do_sfeatures(struct cmd_context
> *ctx)
> if (!any_changed) {
> fprintf(stderr,
> "Could not change any device
> features\n");
> - return 1;
> + goto finish;
> }
> printf("Actual changes:\n");
> dump_features(defs, new_state, old_state);
> }
>
> - return 0;
> + retval = 0;
> +finish:
> + free(new_state);
> + free(old_state);
> + free(efeatures);
> + free(defs);
> + return retval;
> }
>
> static int do_gset(struct cmd_context *ctx)
> @@ -2705,8 +2717,18 @@ static int do_gregs(struct cmd_context *ctx)
> return 75;
> }
>
> - regs = realloc(regs, sizeof(*regs) + st.st_size);
> - regs->len = st.st_size;
> + if (regs->len != st.st_size) {
> + struct ethtool_regs *new_regs;
> + new_regs = realloc(regs, sizeof(*regs) +
> st.st_size);
> + if (!new_regs) {
> + perror("Cannot allocate memory for
> register "
> + "dump");
> + free(regs);
> + return 73;
> + }
> + regs = new_regs;
> + regs->len = st.st_size;
> + }
> nread = fread(regs->data, regs->len, 1, f);
> fclose(f);
> if (nread != 1) {
> @@ -3775,6 +3797,7 @@ static int do_gprivflags(struct cmd_context
> *ctx)
> }
> if (strings->len == 0) {
> fprintf(stderr, "No private flags defined\n");
> + free(strings);
> return 1;
> }
> if (strings->len > 32) {
> @@ -3786,6 +3809,7 @@ static int do_gprivflags(struct cmd_context
> *ctx)
> flags.cmd = ETHTOOL_GPFLAGS;
> if (send_ioctl(ctx, &flags)) {
> perror("Cannot get private flags");
> + free(strings);
> return 1;
> }
>
> @@ -3804,6 +3828,7 @@ static int do_gprivflags(struct cmd_context
> *ctx)
> (const char *)strings->data + i *
> ETH_GSTRING_LEN,
> (flags.data & (1U << i)) ? "on" : "off");
>
> + free(strings);
> return 0;
> }
>
> @@ -3825,6 +3850,7 @@ static int do_sprivflags(struct cmd_context
> *ctx)
> }
> if (strings->len == 0) {
> fprintf(stderr, "No private flags defined\n");
> + free(strings);
> return 1;
> }
> if (strings->len > 32) {
> @@ -3836,6 +3862,7 @@ static int do_sprivflags(struct cmd_context
> *ctx)
> cmdline = calloc(strings->len, sizeof(*cmdline));
> if (!cmdline) {
> perror("Cannot parse arguments");
> + free(strings);
> return 1;
> }
> for (i = 0; i < strings->len; i++) {
> @@ -3852,6 +3879,7 @@ static int do_sprivflags(struct cmd_context
> *ctx)
> flags.cmd = ETHTOOL_GPFLAGS;
> if (send_ioctl(ctx, &flags)) {
> perror("Cannot get private flags");
> + free(strings);
> return 1;
> }
>
> @@ -3859,9 +3887,11 @@ static int do_sprivflags(struct cmd_context
> *ctx)
> flags.data = (flags.data & ~seen_flags) | wanted_flags;
> if (send_ioctl(ctx, &flags)) {
> perror("Cannot set private flags");
> + free(strings);
> return 1;
> }
>
> + free(strings);
> return 0;
> }
>
--
Ben Hutchings
compatible: Gracefully accepts erroneous data from any source
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH ethtool] ethtool.c: fix memory leaks
2016-06-26 8:59 ` Ben Hutchings
@ 2016-06-27 9:41 ` Ivan Vecera
2016-06-27 9:59 ` David Laight
0 siblings, 1 reply; 5+ messages in thread
From: Ivan Vecera @ 2016-06-27 9:41 UTC (permalink / raw)
To: Ben Hutchings, netdev; +Cc: bwh
On 26.6.2016 10:59, Ben Hutchings wrote:
> On Fri, 2016-03-18 at 13:24 +0100, Ivan Vecera wrote:
>> Memory allocated at several places is not appropriately freed.
>
> Given that ethtool is not a library or a long-running application - why
> does that matter?
>
> Ben.
Because each decently written program should clean up. Sure the kernel
will take care about it after exit but we cannot accept the fact that
short-running applications can behave like a pig.
Ivan
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH ethtool] ethtool.c: fix memory leaks
2016-06-27 9:41 ` Ivan Vecera
@ 2016-06-27 9:59 ` David Laight
0 siblings, 0 replies; 5+ messages in thread
From: David Laight @ 2016-06-27 9:59 UTC (permalink / raw)
To: 'Ivan Vecera', Ben Hutchings, netdev; +Cc: bwh
From: Ivan Vecera
> Sent: 27 June 2016 10:41
> On 26.6.2016 10:59, Ben Hutchings wrote:
> > On Fri, 2016-03-18 at 13:24 +0100, Ivan Vecera wrote:
> >> Memory allocated at several places is not appropriately freed.
> >
> > Given that ethtool is not a library or a long-running application - why
> > does that matter?
> >
> > Ben.
>
> Because each decently written program should clean up. Sure the kernel
> will take care about it after exit but we cannot accept the fact that
> short-running applications can behave like a pig.
It really doesn't matter except for memory that might be allocated
repeatedly.
The risks of getting it wrong probably outway any minor benefit.
free() is also unlikely to reduce the program size (to do so would require
a system call and TLB flush on top of everything else).
David
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-06-27 10:01 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-18 12:24 [PATCH ethtool] ethtool.c: fix memory leaks Ivan Vecera
2016-04-08 8:06 ` Ivan Vecera
2016-06-26 8:59 ` Ben Hutchings
2016-06-27 9:41 ` Ivan Vecera
2016-06-27 9:59 ` David Laight
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.