* address some open scan hub warnings @ 2024-08-20 18:26 jmoyer 2024-08-20 18:26 ` [PATCH ndctl 1/2] ndctl/keys.c: don't leak fd in error cases jmoyer 2024-08-20 18:26 ` [PATCH ndctl 2/2] libndctl.c: major and minor numbers are unsigned jmoyer 0 siblings, 2 replies; 9+ messages in thread From: jmoyer @ 2024-08-20 18:26 UTC (permalink / raw) To: nvdimm These issues are pretty minor, but probably worth fixing. [PATCH ndctl 1/2] ndctl/keys.c: don't leak fd in error cases [PATCH ndctl 2/2] libndctl.c: major and minor numbers are unsigned ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH ndctl 1/2] ndctl/keys.c: don't leak fd in error cases 2024-08-20 18:26 address some open scan hub warnings jmoyer @ 2024-08-20 18:26 ` jmoyer 2024-08-20 19:59 ` Dave Jiang 2024-08-21 21:50 ` Verma, Vishal L 2024-08-20 18:26 ` [PATCH ndctl 2/2] libndctl.c: major and minor numbers are unsigned jmoyer 1 sibling, 2 replies; 9+ messages in thread From: jmoyer @ 2024-08-20 18:26 UTC (permalink / raw) To: nvdimm From: Jeff Moyer <jmoyer@redhat.com> Static analysis points out that fd is leaked in some cases. The change to the while loop is optional. I only did that to make the code consistent. Signed-off-by: Jeff Moyer <jmoyer@redhat.com> --- ndctl/keys.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/ndctl/keys.c b/ndctl/keys.c index 2c1f474..cc55204 100644 --- a/ndctl/keys.c +++ b/ndctl/keys.c @@ -108,7 +108,7 @@ char *ndctl_load_key_blob(const char *path, int *size, const char *postfix, struct stat st; ssize_t read_bytes = 0; int rc, fd; - char *blob, *pl, *rdptr; + char *blob = NULL, *pl, *rdptr; char prefix[] = "load "; bool need_prefix = false; @@ -125,16 +125,16 @@ char *ndctl_load_key_blob(const char *path, int *size, const char *postfix, rc = fstat(fd, &st); if (rc < 0) { fprintf(stderr, "stat: %s\n", strerror(errno)); - return NULL; + goto out_close; } if ((st.st_mode & S_IFMT) != S_IFREG) { fprintf(stderr, "%s not a regular file\n", path); - return NULL; + goto out_close; } if (st.st_size == 0 || st.st_size > 4096) { fprintf(stderr, "Invalid blob file size\n"); - return NULL; + goto out_close; } *size = st.st_size; @@ -166,15 +166,13 @@ char *ndctl_load_key_blob(const char *path, int *size, const char *postfix, fprintf(stderr, "Failed to read from blob file: %s\n", strerror(errno)); free(blob); - close(fd); - return NULL; + blob = NULL; + goto out_close; } read_bytes += rc; rdptr += rc; } while (read_bytes != st.st_size); - close(fd); - if (postfix) { pl += read_bytes; *pl = ' '; @@ -182,6 +180,8 @@ char *ndctl_load_key_blob(const char *path, int *size, const char *postfix, rc = sprintf(pl, "keyhandle=%s", postfix); } +out_close: + close(fd); return blob; } -- 2.43.5 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH ndctl 1/2] ndctl/keys.c: don't leak fd in error cases 2024-08-20 18:26 ` [PATCH ndctl 1/2] ndctl/keys.c: don't leak fd in error cases jmoyer @ 2024-08-20 19:59 ` Dave Jiang 2024-08-21 21:50 ` Verma, Vishal L 1 sibling, 0 replies; 9+ messages in thread From: Dave Jiang @ 2024-08-20 19:59 UTC (permalink / raw) To: jmoyer, nvdimm On 8/20/24 11:26 AM, jmoyer@redhat.com wrote: > From: Jeff Moyer <jmoyer@redhat.com> > > Static analysis points out that fd is leaked in some cases. The > change to the while loop is optional. I only did that to make the > code consistent. > > Signed-off-by: Jeff Moyer <jmoyer@redhat.com> Reviewed-by: Dave Jiang <dave.jiang@intel.com> > --- > ndctl/keys.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/ndctl/keys.c b/ndctl/keys.c > index 2c1f474..cc55204 100644 > --- a/ndctl/keys.c > +++ b/ndctl/keys.c > @@ -108,7 +108,7 @@ char *ndctl_load_key_blob(const char *path, int *size, const char *postfix, > struct stat st; > ssize_t read_bytes = 0; > int rc, fd; > - char *blob, *pl, *rdptr; > + char *blob = NULL, *pl, *rdptr; > char prefix[] = "load "; > bool need_prefix = false; > > @@ -125,16 +125,16 @@ char *ndctl_load_key_blob(const char *path, int *size, const char *postfix, > rc = fstat(fd, &st); > if (rc < 0) { > fprintf(stderr, "stat: %s\n", strerror(errno)); > - return NULL; > + goto out_close; > } > if ((st.st_mode & S_IFMT) != S_IFREG) { > fprintf(stderr, "%s not a regular file\n", path); > - return NULL; > + goto out_close; > } > > if (st.st_size == 0 || st.st_size > 4096) { > fprintf(stderr, "Invalid blob file size\n"); > - return NULL; > + goto out_close; > } > > *size = st.st_size; > @@ -166,15 +166,13 @@ char *ndctl_load_key_blob(const char *path, int *size, const char *postfix, > fprintf(stderr, "Failed to read from blob file: %s\n", > strerror(errno)); > free(blob); > - close(fd); > - return NULL; > + blob = NULL; > + goto out_close; > } > read_bytes += rc; > rdptr += rc; > } while (read_bytes != st.st_size); > > - close(fd); > - > if (postfix) { > pl += read_bytes; > *pl = ' '; > @@ -182,6 +180,8 @@ char *ndctl_load_key_blob(const char *path, int *size, const char *postfix, > rc = sprintf(pl, "keyhandle=%s", postfix); > } > > +out_close: > + close(fd); > return blob; > } > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH ndctl 1/2] ndctl/keys.c: don't leak fd in error cases 2024-08-20 18:26 ` [PATCH ndctl 1/2] ndctl/keys.c: don't leak fd in error cases jmoyer 2024-08-20 19:59 ` Dave Jiang @ 2024-08-21 21:50 ` Verma, Vishal L 1 sibling, 0 replies; 9+ messages in thread From: Verma, Vishal L @ 2024-08-21 21:50 UTC (permalink / raw) To: jmoyer@redhat.com, nvdimm@lists.linux.dev On Tue, 2024-08-20 at 14:26 -0400, jmoyer@redhat.com wrote: > From: Jeff Moyer <jmoyer@redhat.com> > > Static analysis points out that fd is leaked in some cases. The > change to the while loop is optional. I only did that to make the > code consistent. > > Signed-off-by: Jeff Moyer <jmoyer@redhat.com> Looks good, Reviewed-by: Vishal Verma <vishal.l.verma@intel.com> > --- > ndctl/keys.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/ndctl/keys.c b/ndctl/keys.c > index 2c1f474..cc55204 100644 > --- a/ndctl/keys.c > +++ b/ndctl/keys.c > @@ -108,7 +108,7 @@ char *ndctl_load_key_blob(const char *path, int > *size, const char *postfix, > struct stat st; > ssize_t read_bytes = 0; > int rc, fd; > - char *blob, *pl, *rdptr; > + char *blob = NULL, *pl, *rdptr; > char prefix[] = "load "; > bool need_prefix = false; > > @@ -125,16 +125,16 @@ char *ndctl_load_key_blob(const char *path, int > *size, const char *postfix, > rc = fstat(fd, &st); > if (rc < 0) { > fprintf(stderr, "stat: %s\n", strerror(errno)); > - return NULL; > + goto out_close; > } > if ((st.st_mode & S_IFMT) != S_IFREG) { > fprintf(stderr, "%s not a regular file\n", path); > - return NULL; > + goto out_close; > } > > if (st.st_size == 0 || st.st_size > 4096) { > fprintf(stderr, "Invalid blob file size\n"); > - return NULL; > + goto out_close; > } > > *size = st.st_size; > @@ -166,15 +166,13 @@ char *ndctl_load_key_blob(const char *path, int > *size, const char *postfix, > fprintf(stderr, "Failed to read from blob > file: %s\n", > strerror(errno)); > free(blob); > - close(fd); > - return NULL; > + blob = NULL; > + goto out_close; > } > read_bytes += rc; > rdptr += rc; > } while (read_bytes != st.st_size); > > - close(fd); > - > if (postfix) { > pl += read_bytes; > *pl = ' '; > @@ -182,6 +180,8 @@ char *ndctl_load_key_blob(const char *path, int > *size, const char *postfix, > rc = sprintf(pl, "keyhandle=%s", postfix); > } > > +out_close: > + close(fd); > return blob; > } > ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH ndctl 2/2] libndctl.c: major and minor numbers are unsigned 2024-08-20 18:26 address some open scan hub warnings jmoyer 2024-08-20 18:26 ` [PATCH ndctl 1/2] ndctl/keys.c: don't leak fd in error cases jmoyer @ 2024-08-20 18:26 ` jmoyer 2024-08-20 19:59 ` Dave Jiang 2024-08-21 21:50 ` Verma, Vishal L 1 sibling, 2 replies; 9+ messages in thread From: jmoyer @ 2024-08-20 18:26 UTC (permalink / raw) To: nvdimm From: Jeff Moyer <jmoyer@redhat.com> Static analysis points out that the cast of bus->major and bus->minor to a signed type in the call to parent_dev_path could result in a negative number. I sincerely doubt we'll see major and minor numbers that large, but let's fix it. Signed-off-by: Jeff Moyer <jmoyer@redhat.com> --- ndctl/lib/libndctl.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c index ddbdd9a..f75dbd4 100644 --- a/ndctl/lib/libndctl.c +++ b/ndctl/lib/libndctl.c @@ -710,11 +710,12 @@ NDCTL_EXPORT void ndctl_set_log_priority(struct ndctl_ctx *ctx, int priority) daxctl_set_log_priority(ctx->daxctl_ctx, priority); } -static char *__dev_path(char *type, int major, int minor, int parent) +static char *__dev_path(char *type, unsigned int major, unsigned int minor, + int parent) { char *path, *dev_path; - if (asprintf(&path, "/sys/dev/%s/%d:%d%s", type, major, minor, + if (asprintf(&path, "/sys/dev/%s/%u:%u%s", type, major, minor, parent ? "/device" : "") < 0) return NULL; @@ -723,7 +724,7 @@ static char *__dev_path(char *type, int major, int minor, int parent) return dev_path; } -static char *parent_dev_path(char *type, int major, int minor) +static char *parent_dev_path(char *type, unsigned int major, unsigned int minor) { return __dev_path(type, major, minor, 1); } -- 2.43.5 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH ndctl 2/2] libndctl.c: major and minor numbers are unsigned 2024-08-20 18:26 ` [PATCH ndctl 2/2] libndctl.c: major and minor numbers are unsigned jmoyer @ 2024-08-20 19:59 ` Dave Jiang 2024-08-21 21:50 ` Verma, Vishal L 1 sibling, 0 replies; 9+ messages in thread From: Dave Jiang @ 2024-08-20 19:59 UTC (permalink / raw) To: jmoyer, nvdimm On 8/20/24 11:26 AM, jmoyer@redhat.com wrote: > From: Jeff Moyer <jmoyer@redhat.com> > > Static analysis points out that the cast of bus->major and bus->minor > to a signed type in the call to parent_dev_path could result in a > negative number. I sincerely doubt we'll see major and minor numbers > that large, but let's fix it. > > Signed-off-by: Jeff Moyer <jmoyer@redhat.com> Reviewed-by: Dave Jiang <dave.jiang@intel.com> > --- > ndctl/lib/libndctl.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c > index ddbdd9a..f75dbd4 100644 > --- a/ndctl/lib/libndctl.c > +++ b/ndctl/lib/libndctl.c > @@ -710,11 +710,12 @@ NDCTL_EXPORT void ndctl_set_log_priority(struct ndctl_ctx *ctx, int priority) > daxctl_set_log_priority(ctx->daxctl_ctx, priority); > } > > -static char *__dev_path(char *type, int major, int minor, int parent) > +static char *__dev_path(char *type, unsigned int major, unsigned int minor, > + int parent) > { > char *path, *dev_path; > > - if (asprintf(&path, "/sys/dev/%s/%d:%d%s", type, major, minor, > + if (asprintf(&path, "/sys/dev/%s/%u:%u%s", type, major, minor, > parent ? "/device" : "") < 0) > return NULL; > > @@ -723,7 +724,7 @@ static char *__dev_path(char *type, int major, int minor, int parent) > return dev_path; > } > > -static char *parent_dev_path(char *type, int major, int minor) > +static char *parent_dev_path(char *type, unsigned int major, unsigned int minor) > { > return __dev_path(type, major, minor, 1); > } ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH ndctl 2/2] libndctl.c: major and minor numbers are unsigned 2024-08-20 18:26 ` [PATCH ndctl 2/2] libndctl.c: major and minor numbers are unsigned jmoyer 2024-08-20 19:59 ` Dave Jiang @ 2024-08-21 21:50 ` Verma, Vishal L 1 sibling, 0 replies; 9+ messages in thread From: Verma, Vishal L @ 2024-08-21 21:50 UTC (permalink / raw) To: jmoyer@redhat.com, nvdimm@lists.linux.dev On Tue, 2024-08-20 at 14:26 -0400, jmoyer@redhat.com wrote: > From: Jeff Moyer <jmoyer@redhat.com> > > Static analysis points out that the cast of bus->major and bus->minor > to a signed type in the call to parent_dev_path could result in a > negative number. I sincerely doubt we'll see major and minor numbers > that large, but let's fix it. > > Signed-off-by: Jeff Moyer <jmoyer@redhat.com> Looks good, Reviewed-by: Vishal Verma <vishal.l.verma@intel.com> > --- > ndctl/lib/libndctl.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c > index ddbdd9a..f75dbd4 100644 > --- a/ndctl/lib/libndctl.c > +++ b/ndctl/lib/libndctl.c > @@ -710,11 +710,12 @@ NDCTL_EXPORT void ndctl_set_log_priority(struct > ndctl_ctx *ctx, int priority) > daxctl_set_log_priority(ctx->daxctl_ctx, priority); > } > > -static char *__dev_path(char *type, int major, int minor, int > parent) > +static char *__dev_path(char *type, unsigned int major, unsigned int > minor, > + int parent) > { > char *path, *dev_path; > > - if (asprintf(&path, "/sys/dev/%s/%d:%d%s", type, major, > minor, > + if (asprintf(&path, "/sys/dev/%s/%u:%u%s", type, major, > minor, > parent ? "/device" : "") < 0) > return NULL; > > @@ -723,7 +724,7 @@ static char *__dev_path(char *type, int major, > int minor, int parent) > return dev_path; > } > > -static char *parent_dev_path(char *type, int major, int minor) > +static char *parent_dev_path(char *type, unsigned int major, > unsigned int minor) > { > return __dev_path(type, major, minor, 1); > } ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH ndctl 0/2] fix errors pointed out by static analysis @ 2024-05-03 20:54 jmoyer 2024-05-03 20:54 ` [PATCH ndctl 2/2] libndctl.c: major and minor numbers are unsigned jmoyer 0 siblings, 1 reply; 9+ messages in thread From: jmoyer @ 2024-05-03 20:54 UTC (permalink / raw) To: nvdimm; +Cc: vishal.l.verma, Jeff Moyer From: Jeff Moyer <jmoyer@redhat.com> This series fixes a couple of minor issues flagged by coverity. Jeff Moyer (2): ndctl/keys.c: don't leak fd in error cases libndctl.c: major and minor numbers are unsigned ndctl/keys.c | 16 ++++++++-------- ndctl/lib/libndctl.c | 7 ++++--- 2 files changed, 12 insertions(+), 11 deletions(-) -- 2.43.0 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH ndctl 2/2] libndctl.c: major and minor numbers are unsigned 2024-05-03 20:54 [PATCH ndctl 0/2] fix errors pointed out by static analysis jmoyer @ 2024-05-03 20:54 ` jmoyer 2024-05-06 19:11 ` Verma, Vishal L 0 siblings, 1 reply; 9+ messages in thread From: jmoyer @ 2024-05-03 20:54 UTC (permalink / raw) To: nvdimm; +Cc: vishal.l.verma, Jeff Moyer From: Jeff Moyer <jmoyer@redhat.com> Static analysis points out that the cast of bus->major and bus->minor to a signed type in the call to parent_dev_path could result in a negative number. I sincerely doubt we'll see major and minor numbers that large, but let's fix it. Signed-off-by: Jeff Moyer <jmoyer@redhat.com> --- ndctl/lib/libndctl.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c index ddbdd9a..f75dbd4 100644 --- a/ndctl/lib/libndctl.c +++ b/ndctl/lib/libndctl.c @@ -710,11 +710,12 @@ NDCTL_EXPORT void ndctl_set_log_priority(struct ndctl_ctx *ctx, int priority) daxctl_set_log_priority(ctx->daxctl_ctx, priority); } -static char *__dev_path(char *type, int major, int minor, int parent) +static char *__dev_path(char *type, unsigned int major, unsigned int minor, + int parent) { char *path, *dev_path; - if (asprintf(&path, "/sys/dev/%s/%d:%d%s", type, major, minor, + if (asprintf(&path, "/sys/dev/%s/%u:%u%s", type, major, minor, parent ? "/device" : "") < 0) return NULL; @@ -723,7 +724,7 @@ static char *__dev_path(char *type, int major, int minor, int parent) return dev_path; } -static char *parent_dev_path(char *type, int major, int minor) +static char *parent_dev_path(char *type, unsigned int major, unsigned int minor) { return __dev_path(type, major, minor, 1); } -- 2.43.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH ndctl 2/2] libndctl.c: major and minor numbers are unsigned 2024-05-03 20:54 ` [PATCH ndctl 2/2] libndctl.c: major and minor numbers are unsigned jmoyer @ 2024-05-06 19:11 ` Verma, Vishal L 0 siblings, 0 replies; 9+ messages in thread From: Verma, Vishal L @ 2024-05-06 19:11 UTC (permalink / raw) To: jmoyer@redhat.com, nvdimm@lists.linux.dev On Fri, 2024-05-03 at 16:54 -0400, jmoyer@redhat.com wrote: > From: Jeff Moyer <jmoyer@redhat.com> > > Static analysis points out that the cast of bus->major and bus->minor > to a signed type in the call to parent_dev_path could result in a > negative number. I sincerely doubt we'll see major and minor numbers > that large, but let's fix it. > > Signed-off-by: Jeff Moyer <jmoyer@redhat.com> Looks good, Reviewed-by: Vishal Verma <vishal.l.verma@intel.com> > --- > ndctl/lib/libndctl.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c > index ddbdd9a..f75dbd4 100644 > --- a/ndctl/lib/libndctl.c > +++ b/ndctl/lib/libndctl.c > @@ -710,11 +710,12 @@ NDCTL_EXPORT void ndctl_set_log_priority(struct > ndctl_ctx *ctx, int priority) > daxctl_set_log_priority(ctx->daxctl_ctx, priority); > } > > -static char *__dev_path(char *type, int major, int minor, int > parent) > +static char *__dev_path(char *type, unsigned int major, unsigned int > minor, > + int parent) > { > char *path, *dev_path; > > - if (asprintf(&path, "/sys/dev/%s/%d:%d%s", type, major, > minor, > + if (asprintf(&path, "/sys/dev/%s/%u:%u%s", type, major, > minor, > parent ? "/device" : "") < 0) > return NULL; > > @@ -723,7 +724,7 @@ static char *__dev_path(char *type, int major, > int minor, int parent) > return dev_path; > } > > -static char *parent_dev_path(char *type, int major, int minor) > +static char *parent_dev_path(char *type, unsigned int major, > unsigned int minor) > { > return __dev_path(type, major, minor, 1); > } ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-08-21 21:50 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-08-20 18:26 address some open scan hub warnings jmoyer 2024-08-20 18:26 ` [PATCH ndctl 1/2] ndctl/keys.c: don't leak fd in error cases jmoyer 2024-08-20 19:59 ` Dave Jiang 2024-08-21 21:50 ` Verma, Vishal L 2024-08-20 18:26 ` [PATCH ndctl 2/2] libndctl.c: major and minor numbers are unsigned jmoyer 2024-08-20 19:59 ` Dave Jiang 2024-08-21 21:50 ` Verma, Vishal L -- strict thread matches above, loose matches on Subject: below -- 2024-05-03 20:54 [PATCH ndctl 0/2] fix errors pointed out by static analysis jmoyer 2024-05-03 20:54 ` [PATCH ndctl 2/2] libndctl.c: major and minor numbers are unsigned jmoyer 2024-05-06 19:11 ` Verma, Vishal L
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox