diff mbox series

misc: fix string buffer compile warnings

Message ID 20240312161242.GA1927156@frogsfrogsfrogs (mailing list archive)
State New
Headers show
Series misc: fix string buffer compile warnings | expand

Commit Message

Darrick J. Wong March 12, 2024, 4:12 p.m. UTC 
From: Darrick J. Wong <djwong@kernel.org>

Fix various minor compiler warnings (gcc 12.2, Debian 12):

Widen various char arrays so that we don't get complaints about buffer
overflows when doing snprintf.

Use snprintf over sprintf.

Use memcpy instead of strncpy when copying fixed-width label fields.

Fix an unused variable warning.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 common/global.c   |    2 +-
 dump/content.c    |    4 ++--
 invutil/fstab.c   |    2 +-
 invutil/invidx.c  |    4 ++--
 invutil/menu.c    |    2 +-
 invutil/stobj.c   |    6 +++---
 restore/content.c |    5 ++---
 7 files changed, 12 insertions(+), 13 deletions(-)

Comments

Christoph Hellwig March 12, 2024, 9:20 p.m. UTC | #1 
Funnily enough I just started looking into xfsdump warnings a few
minutes ago..

> --- a/common/global.c
> +++ b/common/global.c
> @@ -82,7 +82,7 @@ global_hdr_alloc(int argc, char *argv[])
>  
>  	/* fill in the magic number
>  	 */
> -	strncpy(ghdrp->gh_magic, GLOBAL_HDR_MAGIC, GLOBAL_HDR_MAGIC_SZ);
> +	memcpy(ghdrp->gh_magic, GLOBAL_HDR_MAGIC, GLOBAL_HDR_MAGIC_SZ);

This chunk and all the other ones switching to memcpy where we have
a fixed size look good and impossible to improve on to me.

> -	sprintf(question,
> +	snprintf(question, sizeof(question),
>  		 "pre-erase (-%c) option specified "
>  		 "and non-blank media encountered:\n"
>  		 "please confirm media erase "
> diff --git a/invutil/fstab.c b/invutil/fstab.c

For this and a few others that just s(n)printf I wonder if just
switching to asprintf and dynamically allocating the buffer is
the right thing to do.  That's a GNU/BSD extension, but we probably
don't care about anything else.

> index 88d849e..56132e1 100644
> --- a/invutil/fstab.c
> +++ b/invutil/fstab.c
> @@ -149,7 +149,7 @@ fstab_select(WINDOW *win, node_t *current, node_t *list)
>  int
>  fstab_highlight(WINDOW *win, node_t *current, node_t *list)
>  {
> -    static char txt[256];
> +    static char txt[512];

And for put_info_line/put_line I suspect just passing a format
string is the best thing to do, as this avoids the extra
snprintf and buffer entirely.  That's in fact what I had just started
on.
diff mbox series

Patch

diff --git a/common/global.c b/common/global.c
index 6a4e348..53fb4e0 100644
--- a/common/global.c
+++ b/common/global.c
@@ -82,7 +82,7 @@  global_hdr_alloc(int argc, char *argv[])
 
 	/* fill in the magic number
 	 */
-	strncpy(ghdrp->gh_magic, GLOBAL_HDR_MAGIC, GLOBAL_HDR_MAGIC_SZ);
+	memcpy(ghdrp->gh_magic, GLOBAL_HDR_MAGIC, GLOBAL_HDR_MAGIC_SZ);
 
 	/* fill in the hdr version
 	 */
diff --git a/dump/content.c b/dump/content.c
index 6462267..a0d3a84 100644
--- a/dump/content.c
+++ b/dump/content.c
@@ -6348,7 +6348,7 @@  static bool_t
 Media_prompt_erase(drive_t *drivep)
 {
 	fold_t fold;
-	char question[100];
+	char question[256];
 	char *preamblestr[PREAMBLEMAX];
 	size_t preamblecnt;
 	char *querystr[QUERYMAX];
@@ -6375,7 +6375,7 @@  Media_prompt_erase(drive_t *drivep)
 
 	/* query: ask if overwrite ok
 	 */
-	sprintf(question,
+	snprintf(question, sizeof(question),
 		 "pre-erase (-%c) option specified "
 		 "and non-blank media encountered:\n"
 		 "please confirm media erase "
diff --git a/invutil/fstab.c b/invutil/fstab.c
index 88d849e..56132e1 100644
--- a/invutil/fstab.c
+++ b/invutil/fstab.c
@@ -149,7 +149,7 @@  fstab_select(WINDOW *win, node_t *current, node_t *list)
 int
 fstab_highlight(WINDOW *win, node_t *current, node_t *list)
 {
-    static char txt[256];
+    static char txt[512];
     char uuidstr[UUID_STR_LEN + 1];
     data_t *d;
     invt_fstab_t *fstabentry;
diff --git a/invutil/invidx.c b/invutil/invidx.c
index 5874e8d..0fb55c5 100644
--- a/invutil/invidx.c
+++ b/invutil/invidx.c
@@ -115,7 +115,7 @@  invidx_commit(WINDOW *win, node_t *current, node_t *list)
 	struct stat s;
 	char dst_stobjfile[MAXPATHLEN];
 	char *stobjfile;
-	char cmd[1024];
+	char cmd[(MAXPATHLEN * 2) + 16];
 
 	snprintf(dst_idxfile, sizeof(dst_idxfile), "%s/%s", inventory_path, basename(invidx_file[fidx].name));
 
@@ -688,7 +688,7 @@  invidx_select(WINDOW *win, node_t *current, node_t *list)
 int
 invidx_highlight(WINDOW *win, node_t *current, node_t *list)
 {
-    static char txt[256];
+    static char txt[512];
     data_t *d;
     invt_entry_t *invtentry;
 
diff --git a/invutil/menu.c b/invutil/menu.c
index 81baa42..89a350c 100644
--- a/invutil/menu.c
+++ b/invutil/menu.c
@@ -101,7 +101,7 @@  menu(WINDOW *win, int line, int col, node_t *list, int keyc, menukey_t *keyv)
     int c;
     int k;
     int quit;
-    char txt[256];
+    char txt[512];
     node_t *current;
     node_t *last;
     data_t *d;
diff --git a/invutil/stobj.c b/invutil/stobj.c
index 2912e0c..852f160 100644
--- a/invutil/stobj.c
+++ b/invutil/stobj.c
@@ -171,7 +171,7 @@  stobjsess_commit(WINDOW *win, node_t *current, node_t *list)
 int
 stobjsess_highlight(WINDOW *win, node_t *current, node_t *list)
 {
-    static char txt[256];
+    static char txt[512];
     char uuidstr[UUID_STR_LEN + 1];
     data_t *d;
     invt_seshdr_t *stobjhdr;
@@ -210,7 +210,7 @@  stobjsess_highlight(WINDOW *win, node_t *current, node_t *list)
 int
 stobjstrm_highlight(WINDOW *win, node_t *current, node_t *list)
 {
-    static char txt[256];
+    static char txt[512];
     data_t *d;
     invt_stream_t *stobjstrm;
 
@@ -246,7 +246,7 @@  stobjstrm_highlight(WINDOW *win, node_t *current, node_t *list)
 int
 stobjmed_highlight(WINDOW *win, node_t *current, node_t *list)
 {
-    static char txt[256];
+    static char txt[512];
     char uuidstr[UUID_STR_LEN + 1];
     data_t *d;
     invt_mediafile_t *stobjmed;
diff --git a/restore/content.c b/restore/content.c
index 7ec3a4d..5536411 100644
--- a/restore/content.c
+++ b/restore/content.c
@@ -5077,7 +5077,7 @@  pi_insertfile(ix_t drivecnt,
 	     &&
 	     ! DH2O(objh)->o_idlabvalpr) {
 		uuid_copy(DH2O(objh)->o_id, *mediaidp);
-		strncpy(DH2O(objh)->o_lab,
+		memcpy(DH2O(objh)->o_lab,
 			 medialabel,
 			 sizeof(DH2O(objh)->o_lab));
 		DH2O(objh)->o_idlabvalpr = BOOL_TRUE;
@@ -5107,7 +5107,7 @@  pi_insertfile(ix_t drivecnt,
 	     &&
 	     ! DH2O(prevobjh)->o_idlabvalpr) {
 		uuid_copy(DH2O(prevobjh)->o_id, *prevmediaidp);
-		strncpy(DH2O(prevobjh)->o_lab,
+		memcpy(DH2O(prevobjh)->o_lab,
 			       prevmedialabel,
 			       sizeof(DH2O(prevobjh)->o_lab));
 		DH2O(prevobjh)->o_idlabvalpr = BOOL_TRUE;
@@ -8632,7 +8632,6 @@  restore_extattr(drive_t *drivep,
 {
 	drive_ops_t *dop = drivep->d_opsp;
 	extattrhdr_t *ahdrp = (extattrhdr_t *)get_extattrbuf(drivep->d_index);
-	stream_context_t *strctxp = (stream_context_t *)drivep->d_strmcontextp;
 	bstat_t *bstatp = &fhdrp->fh_stat;
 	bool_t isfilerestored = BOOL_FALSE;