diff mbox series

[ndctl] util/strbuf: remove unused cli infrastructure imports

Message ID 20250212034020.1865719-1-alison.schofield@intel.com (mailing list archive)
State New
Headers show
Series [ndctl] util/strbuf: remove unused cli infrastructure imports | expand

Commit Message

Alison Schofield Feb. 12, 2025, 3:40 a.m. UTC 
From: Alison Schofield <alison.schofield@intel.com>

The ndctl cli interface is built around an imported perf cli
infrastructure which was originally from git. [1]

A recent static analysis scan exposed an integer overflow issue in
sysbuf_read() and although that is fixable, the function is not used
in ndctl. Further examination revealed additional unused functionality
in the string buffer handling import and a subset of that has already
been obsoleted from the perf cli.

In the interest of not maintaining unused code, remove the unused code
in util/strbuf.h,c. Ndctl, including cxl-cli and daxctl, are mature
cli's so it seems ok to let this functionality go after 14 years.

In the interest of not touching what is not causing an issue, the
entirety of the original import was not reviewed at this time.

[1] 91677390f9e6 ("ndctl: import cli infrastructure from perf")

Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
 util/strbuf.c | 51 ---------------------------------------------------
 util/strbuf.h |  7 -------
 2 files changed, 58 deletions(-)


base-commit: a3d56f0ca5679b7c78090c1a8b0b9f1f9901e5e0

Comments

Alison Schofield Feb. 12, 2025, 4:01 p.m. UTC | #1 
On Tue, Feb 11, 2025 at 07:40:18PM -0800, alison.schofield@intel.com wrote:
> From: Alison Schofield <alison.schofield@intel.com>
> 
> The ndctl cli interface is built around an imported perf cli
> infrastructure which was originally from git. [1]
> 
> A recent static analysis scan exposed an integer overflow issue in
> sysbuf_read() and although that is fixable, the function is not used
  ^^^
  strbuf_read()


> in ndctl. Further examination revealed additional unused functionality
> in the string buffer handling import and a subset of that has already
> been obsoleted from the perf cli.
> 
> In the interest of not maintaining unused code, remove the unused code
> in util/strbuf.h,c. Ndctl, including cxl-cli and daxctl, are mature
> cli's so it seems ok to let this functionality go after 14 years.
> 
> In the interest of not touching what is not causing an issue, the
> entirety of the original import was not reviewed at this time.
> 
> [1] 91677390f9e6 ("ndctl: import cli infrastructure from perf")
> 
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> ---
>  util/strbuf.c | 51 ---------------------------------------------------
>  util/strbuf.h |  7 -------
>  2 files changed, 58 deletions(-)

snip
Dave Jiang Feb. 12, 2025, 4:10 p.m. UTC | #2 
On 2/11/25 8:40 PM, alison.schofield@intel.com wrote:
> From: Alison Schofield <alison.schofield@intel.com>
> 
> The ndctl cli interface is built around an imported perf cli
> infrastructure which was originally from git. [1]
> 
> A recent static analysis scan exposed an integer overflow issue in
> sysbuf_read() and although that is fixable, the function is not used
> in ndctl. Further examination revealed additional unused functionality
> in the string buffer handling import and a subset of that has already
> been obsoleted from the perf cli.
> 
> In the interest of not maintaining unused code, remove the unused code
> in util/strbuf.h,c. Ndctl, including cxl-cli and daxctl, are mature
> cli's so it seems ok to let this functionality go after 14 years.
> 
> In the interest of not touching what is not causing an issue, the
> entirety of the original import was not reviewed at this time.
> 
> [1] 91677390f9e6 ("ndctl: import cli infrastructure from perf")
> 
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  util/strbuf.c | 51 ---------------------------------------------------
>  util/strbuf.h |  7 -------
>  2 files changed, 58 deletions(-)
> 
> diff --git a/util/strbuf.c b/util/strbuf.c
> index 6c8752562720..16fc847dd1c7 100644
> --- a/util/strbuf.c
> +++ b/util/strbuf.c
> @@ -60,30 +60,6 @@ void strbuf_grow(struct strbuf *sb, size_t extra)
>  	ALLOC_GROW(sb->buf, sb->len + extra + 1, sb->alloc);
>  }
>  
> -static void strbuf_splice(struct strbuf *sb, size_t pos, size_t len,
> -				   const void *data, size_t dlen)
> -{
> -	if (pos + len < pos)
> -		die("you want to use way too much memory");
> -	if (pos > sb->len)
> -		die("`pos' is too far after the end of the buffer");
> -	if (pos + len > sb->len)
> -		die("`pos + len' is too far after the end of the buffer");
> -
> -	if (dlen >= len)
> -		strbuf_grow(sb, dlen - len);
> -	memmove(sb->buf + pos + dlen,
> -			sb->buf + pos + len,
> -			sb->len - pos - len);
> -	memcpy(sb->buf + pos, data, dlen);
> -	strbuf_setlen(sb, sb->len + dlen - len);
> -}
> -
> -void strbuf_remove(struct strbuf *sb, size_t pos, size_t len)
> -{
> -	strbuf_splice(sb, pos, len, NULL, 0);
> -}
> -
>  void strbuf_add(struct strbuf *sb, const void *data, size_t len)
>  {
>  	strbuf_grow(sb, len);
> @@ -114,30 +90,3 @@ void strbuf_addf(struct strbuf *sb, const char *fmt, ...)
>  	}
>  	strbuf_setlen(sb, sb->len + len);
>  }
> -
> -ssize_t strbuf_read(struct strbuf *sb, int fd, ssize_t hint)
> -{
> -	size_t oldlen = sb->len;
> -	size_t oldalloc = sb->alloc;
> -
> -	strbuf_grow(sb, hint ? hint : 8192);
> -	for (;;) {
> -		ssize_t cnt;
> -
> -		cnt = read(fd, sb->buf + sb->len, sb->alloc - sb->len - 1);
> -		if (cnt < 0) {
> -			if (oldalloc == 0)
> -				strbuf_release(sb);
> -			else
> -				strbuf_setlen(sb, oldlen);
> -			return -1;
> -		}
> -		if (!cnt)
> -			break;
> -		sb->len += cnt;
> -		strbuf_grow(sb, 8192);
> -	}
> -
> -	sb->buf[sb->len] = '\0';
> -	return sb->len - oldlen;
> -}
> diff --git a/util/strbuf.h b/util/strbuf.h
> index c9b7d2ef5cf8..3f810a5de8d7 100644
> --- a/util/strbuf.h
> +++ b/util/strbuf.h
> @@ -56,7 +56,6 @@ struct strbuf {
>  #define STRBUF_INIT  { 0, 0, strbuf_slopbuf }
>  
>  /*----- strbuf life cycle -----*/
> -extern void strbuf_init(struct strbuf *buf, ssize_t hint);
>  extern void strbuf_release(struct strbuf *);
>  extern char *strbuf_detach(struct strbuf *, size_t *);
>  
> @@ -81,9 +80,6 @@ static inline void strbuf_addch(struct strbuf *sb, int c) {
>  	sb->buf[sb->len++] = c;
>  	sb->buf[sb->len] = '\0';
>  }
> -
> -extern void strbuf_remove(struct strbuf *, size_t pos, size_t len);
> -
>  extern void strbuf_add(struct strbuf *, const void *, size_t);
>  static inline void strbuf_addstr(struct strbuf *sb, const char *s) {
>  	strbuf_add(sb, s, strlen(s));
> @@ -92,7 +88,4 @@ static inline void strbuf_addstr(struct strbuf *sb, const char *s) {
>  __attribute__((format(printf,2,3)))
>  extern void strbuf_addf(struct strbuf *sb, const char *fmt, ...);
>  
> -/* XXX: if read fails, any partial read is undone */
> -extern ssize_t strbuf_read(struct strbuf *, int fd, ssize_t hint);
> -
>  #endif /* __NDCTL_STRBUF_H */
> 
> base-commit: a3d56f0ca5679b7c78090c1a8b0b9f1f9901e5e0
diff mbox series

Patch

diff --git a/util/strbuf.c b/util/strbuf.c
index 6c8752562720..16fc847dd1c7 100644
--- a/util/strbuf.c
+++ b/util/strbuf.c
@@ -60,30 +60,6 @@  void strbuf_grow(struct strbuf *sb, size_t extra)
 	ALLOC_GROW(sb->buf, sb->len + extra + 1, sb->alloc);
 }
 
-static void strbuf_splice(struct strbuf *sb, size_t pos, size_t len,
-				   const void *data, size_t dlen)
-{
-	if (pos + len < pos)
-		die("you want to use way too much memory");
-	if (pos > sb->len)
-		die("`pos' is too far after the end of the buffer");
-	if (pos + len > sb->len)
-		die("`pos + len' is too far after the end of the buffer");
-
-	if (dlen >= len)
-		strbuf_grow(sb, dlen - len);
-	memmove(sb->buf + pos + dlen,
-			sb->buf + pos + len,
-			sb->len - pos - len);
-	memcpy(sb->buf + pos, data, dlen);
-	strbuf_setlen(sb, sb->len + dlen - len);
-}
-
-void strbuf_remove(struct strbuf *sb, size_t pos, size_t len)
-{
-	strbuf_splice(sb, pos, len, NULL, 0);
-}
-
 void strbuf_add(struct strbuf *sb, const void *data, size_t len)
 {
 	strbuf_grow(sb, len);
@@ -114,30 +90,3 @@  void strbuf_addf(struct strbuf *sb, const char *fmt, ...)
 	}
 	strbuf_setlen(sb, sb->len + len);
 }
-
-ssize_t strbuf_read(struct strbuf *sb, int fd, ssize_t hint)
-{
-	size_t oldlen = sb->len;
-	size_t oldalloc = sb->alloc;
-
-	strbuf_grow(sb, hint ? hint : 8192);
-	for (;;) {
-		ssize_t cnt;
-
-		cnt = read(fd, sb->buf + sb->len, sb->alloc - sb->len - 1);
-		if (cnt < 0) {
-			if (oldalloc == 0)
-				strbuf_release(sb);
-			else
-				strbuf_setlen(sb, oldlen);
-			return -1;
-		}
-		if (!cnt)
-			break;
-		sb->len += cnt;
-		strbuf_grow(sb, 8192);
-	}
-
-	sb->buf[sb->len] = '\0';
-	return sb->len - oldlen;
-}
diff --git a/util/strbuf.h b/util/strbuf.h
index c9b7d2ef5cf8..3f810a5de8d7 100644
--- a/util/strbuf.h
+++ b/util/strbuf.h
@@ -56,7 +56,6 @@  struct strbuf {
 #define STRBUF_INIT  { 0, 0, strbuf_slopbuf }
 
 /*----- strbuf life cycle -----*/
-extern void strbuf_init(struct strbuf *buf, ssize_t hint);
 extern void strbuf_release(struct strbuf *);
 extern char *strbuf_detach(struct strbuf *, size_t *);
 
@@ -81,9 +80,6 @@  static inline void strbuf_addch(struct strbuf *sb, int c) {
 	sb->buf[sb->len++] = c;
 	sb->buf[sb->len] = '\0';
 }
-
-extern void strbuf_remove(struct strbuf *, size_t pos, size_t len);
-
 extern void strbuf_add(struct strbuf *, const void *, size_t);
 static inline void strbuf_addstr(struct strbuf *sb, const char *s) {
 	strbuf_add(sb, s, strlen(s));
@@ -92,7 +88,4 @@  static inline void strbuf_addstr(struct strbuf *sb, const char *s) {
 __attribute__((format(printf,2,3)))
 extern void strbuf_addf(struct strbuf *sb, const char *fmt, ...);
 
-/* XXX: if read fails, any partial read is undone */
-extern ssize_t strbuf_read(struct strbuf *, int fd, ssize_t hint);
-
 #endif /* __NDCTL_STRBUF_H */