* [ndctl PATCH] ndctl, monitor: fix a resource leak in parse_monitor_event
@ 2018-07-25 0:09 Vishal Verma
2018-07-25 1:46 ` Qi, Fuli
0 siblings, 1 reply; 2+ messages in thread
From: Vishal Verma @ 2018-07-25 0:09 UTC (permalink / raw)
To: linux-nvdimm
Static analysis reports we leak dimm_event in the above function.
Fix it by adding an 'out' label for all exit paths, and refactoring out
the 'all events' cases.
Fixes: fdf6b6844ccf ("ndctl, monitor: add a new command - monitor")
Cc: QI Fuli <qi.fuli@jp.fujitsu.com>
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
ndctl/monitor.c | 37 ++++++++++++++++++++++---------------
1 file changed, 22 insertions(+), 15 deletions(-)
diff --git a/ndctl/monitor.c b/ndctl/monitor.c
index dbad7aa..b97d1ea 100644
--- a/ndctl/monitor.c
+++ b/ndctl/monitor.c
@@ -410,22 +410,35 @@ static int monitor_event(struct ndctl_ctx *ctx,
return rc;
}
+static void monitor_enable_all_events(struct monitor *_monitor)
+{
+ _monitor->event_flags = ND_EVENT_SPARES_REMAINING
+ | ND_EVENT_MEDIA_TEMPERATURE
+ | ND_EVENT_CTRL_TEMPERATURE
+ | ND_EVENT_HEALTH_STATE
+ | ND_EVENT_UNCLEAN_SHUTDOWN;
+}
+
static int parse_monitor_event(struct monitor *_monitor, struct ndctl_ctx *ctx)
{
char *dimm_event, *save;
const char *event;
+ int rc = 0;
+
+ if (!_monitor->dimm_event) {
+ monitor_enable_all_events(_monitor);
+ return 0;;
+ }
- if (!_monitor->dimm_event)
- goto dimm_event_all;
dimm_event = strdup(_monitor->dimm_event);
if (!dimm_event)
- return 1;
+ return -ENOMEM;
for (event = strtok_r(dimm_event, " ", &save); event;
event = strtok_r(NULL, " ", &save)) {
if (strcmp(event, "all") == 0) {
- free(dimm_event);
- goto dimm_event_all;
+ monitor_enable_all_events(_monitor);
+ goto out;
}
if (strcmp(event, "dimm-spares-remaining") == 0)
_monitor->event_flags |= ND_EVENT_SPARES_REMAINING;
@@ -439,20 +452,14 @@ static int parse_monitor_event(struct monitor *_monitor, struct ndctl_ctx *ctx)
_monitor->event_flags |= ND_EVENT_UNCLEAN_SHUTDOWN;
else {
err(ctx, "no dimm-event named %s\n", event);
- return 1;
+ rc = -EINVAL;
+ goto out;
}
}
+out:
free(dimm_event);
- return 0;
-
-dimm_event_all:
- _monitor->event_flags = ND_EVENT_SPARES_REMAINING
- | ND_EVENT_MEDIA_TEMPERATURE
- | ND_EVENT_CTRL_TEMPERATURE
- | ND_EVENT_HEALTH_STATE
- | ND_EVENT_UNCLEAN_SHUTDOWN;
- return 0;
+ return rc;
}
static void parse_config(const char **arg, char *key, char *val, char *ident)
--
2.14.4
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply related [flat|nested] 2+ messages in thread* RE: [ndctl PATCH] ndctl, monitor: fix a resource leak in parse_monitor_event
2018-07-25 0:09 [ndctl PATCH] ndctl, monitor: fix a resource leak in parse_monitor_event Vishal Verma
@ 2018-07-25 1:46 ` Qi, Fuli
0 siblings, 0 replies; 2+ messages in thread
From: Qi, Fuli @ 2018-07-25 1:46 UTC (permalink / raw)
To: 'Vishal Verma', linux-nvdimm
> -----Original Message-----
> From: Vishal Verma [mailto:vishal.l.verma@intel.com]
> Sent: Wednesday, July 25, 2018 9:09 AM
> To: linux-nvdimm@lists.01.org
> Cc: Qi, Fuli/斉 福利 <qi.fuli@jp.fujitsu.com>; Vishal Verma
> <vishal.l.verma@intel.com>
> Subject: [ndctl PATCH] ndctl, monitor: fix a resource leak in parse_monitor_event
>
> Static analysis reports we leak dimm_event in the above function.
> Fix it by adding an 'out' label for all exit paths, and refactoring out the 'all
> events' cases.
>
> Fixes: fdf6b6844ccf ("ndctl, monitor: add a new command - monitor")
> Cc: QI Fuli <qi.fuli@jp.fujitsu.com>
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> ---
> ndctl/monitor.c | 37 ++++++++++++++++++++++---------------
> 1 file changed, 22 insertions(+), 15 deletions(-)
>
> diff --git a/ndctl/monitor.c b/ndctl/monitor.c index dbad7aa..b97d1ea 100644
> --- a/ndctl/monitor.c
> +++ b/ndctl/monitor.c
> @@ -410,22 +410,35 @@ static int monitor_event(struct ndctl_ctx *ctx,
> return rc;
> }
>
> +static void monitor_enable_all_events(struct monitor *_monitor) {
> + _monitor->event_flags = ND_EVENT_SPARES_REMAINING
> + | ND_EVENT_MEDIA_TEMPERATURE
> + | ND_EVENT_CTRL_TEMPERATURE
> + | ND_EVENT_HEALTH_STATE
> + | ND_EVENT_UNCLEAN_SHUTDOWN;
> +}
> +
> static int parse_monitor_event(struct monitor *_monitor, struct ndctl_ctx *ctx)
> {
> char *dimm_event, *save;
> const char *event;
> + int rc = 0;
> +
> + if (!_monitor->dimm_event) {
> + monitor_enable_all_events(_monitor);
> + return 0;;
> + }
>
> - if (!_monitor->dimm_event)
> - goto dimm_event_all;
> dimm_event = strdup(_monitor->dimm_event);
> if (!dimm_event)
> - return 1;
> + return -ENOMEM;
>
> for (event = strtok_r(dimm_event, " ", &save); event;
> event = strtok_r(NULL, " ", &save)) {
> if (strcmp(event, "all") == 0) {
> - free(dimm_event);
> - goto dimm_event_all;
> + monitor_enable_all_events(_monitor);
> + goto out;
> }
> if (strcmp(event, "dimm-spares-remaining") == 0)
> _monitor->event_flags |= ND_EVENT_SPARES_REMAINING; @@
> -439,20 +452,14 @@ static int parse_monitor_event(struct monitor *_monitor, struct
> ndctl_ctx *ctx)
> _monitor->event_flags |= ND_EVENT_UNCLEAN_SHUTDOWN;
> else {
> err(ctx, "no dimm-event named %s\n", event);
> - return 1;
> + rc = -EINVAL;
> + goto out;
> }
> }
>
> +out:
> free(dimm_event);
> - return 0;
> -
> -dimm_event_all:
> - _monitor->event_flags = ND_EVENT_SPARES_REMAINING
> - | ND_EVENT_MEDIA_TEMPERATURE
> - | ND_EVENT_CTRL_TEMPERATURE
> - | ND_EVENT_HEALTH_STATE
> - | ND_EVENT_UNCLEAN_SHUTDOWN;
> - return 0;
> + return rc;
> }
>
Looks good to me.
Please feel free to add: Reviewed-by: QI Fuli <qi.fuli@jp.fujitsu.com>
Thanks,
QI
> static void parse_config(const char **arg, char *key, char *val, char *ident)
> --
> 2.14.4
>
>
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2018-07-25 1:46 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-25 0:09 [ndctl PATCH] ndctl, monitor: fix a resource leak in parse_monitor_event Vishal Verma
2018-07-25 1:46 ` Qi, Fuli
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).