| Message ID | e3ffd59123f23f53c0bee930ef7602acf2d800c2.1729502824.git.ps@pks.im (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Memory leak fixes (pt.9) | expand |
On 24/10/21 11:28AM, Patrick Steinhardt wrote: > Fix leaking trailer values when replacing the value with a command or > when the token value is empty. > > This leak is exposed by t7513, but plugging it does not make the whole > test suite pass. > > Signed-off-by: Patrick Steinhardt <ps@pks.im> > --- > trailer.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/trailer.c b/trailer.c > index 682d74505bf..f1eca6d5d15 100644 > --- a/trailer.c > +++ b/trailer.c > @@ -249,17 +249,23 @@ static char *apply_command(struct conf_info *conf, const char *arg) > static void apply_item_command(struct trailer_item *in_tok, struct arg_item *arg_tok) > { > if (arg_tok->conf.command || arg_tok->conf.cmd) { > - const char *arg; > + char *value_to_free = NULL; > + char *arg; > + > if (arg_tok->value && arg_tok->value[0]) { > - arg = arg_tok->value; > + arg = (char *)arg_tok->value; Naive question, is this cast not redundant? From looking at `struct arg_item`, `value` already looks to be this type. > } else { > if (in_tok && in_tok->value) > arg = xstrdup(in_tok->value); > else > arg = xstrdup(""); > + value_to_free = arg_tok->value; > } > + > arg_tok->value = apply_command(&arg_tok->conf, arg); > - free((char *)arg); > + > + free(value_to_free); > + free(arg); > } > } > > -- > 2.47.0.72.gef8ce8f3d4.dirty > >
On Mon, Nov 04, 2024 at 04:25:38PM -0600, Justin Tobler wrote: > On 24/10/21 11:28AM, Patrick Steinhardt wrote: > > Fix leaking trailer values when replacing the value with a command or > > when the token value is empty. > > > > This leak is exposed by t7513, but plugging it does not make the whole > > test suite pass. > > > > Signed-off-by: Patrick Steinhardt <ps@pks.im> > > --- > > trailer.c | 12 +++++++++--- > > 1 file changed, 9 insertions(+), 3 deletions(-) > > > > diff --git a/trailer.c b/trailer.c > > index 682d74505bf..f1eca6d5d15 100644 > > --- a/trailer.c > > +++ b/trailer.c > > @@ -249,17 +249,23 @@ static char *apply_command(struct conf_info *conf, const char *arg) > > static void apply_item_command(struct trailer_item *in_tok, struct arg_item *arg_tok) > > { > > if (arg_tok->conf.command || arg_tok->conf.cmd) { > > - const char *arg; > > + char *value_to_free = NULL; > > + char *arg; > > + > > if (arg_tok->value && arg_tok->value[0]) { > > - arg = arg_tok->value; > > + arg = (char *)arg_tok->value; > > Naive question, is this cast not redundant? From looking at `struct > arg_item`, `value` already looks to be this type. Indeed it is, good catch! Patrick
diff --git a/trailer.c b/trailer.c index 682d74505bf..f1eca6d5d15 100644 --- a/trailer.c +++ b/trailer.c @@ -249,17 +249,23 @@ static char *apply_command(struct conf_info *conf, const char *arg) static void apply_item_command(struct trailer_item *in_tok, struct arg_item *arg_tok) { if (arg_tok->conf.command || arg_tok->conf.cmd) { - const char *arg; + char *value_to_free = NULL; + char *arg; + if (arg_tok->value && arg_tok->value[0]) { - arg = arg_tok->value; + arg = (char *)arg_tok->value; } else { if (in_tok && in_tok->value) arg = xstrdup(in_tok->value); else arg = xstrdup(""); + value_to_free = arg_tok->value; } + arg_tok->value = apply_command(&arg_tok->conf, arg); - free((char *)arg); + + free(value_to_free); + free(arg); } }
Fix leaking trailer values when replacing the value with a command or when the token value is empty. This leak is exposed by t7513, but plugging it does not make the whole test suite pass. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- trailer.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)