[PATCH 0/4] xfsprogs: coverity fixes

[PATCH 0/4] xfsprogs: coverity fixes

[Bill O'Donnell]

Select fixes made following coverity scan results on 05/23/2024.
CID: 1596597, 1596598, 1596599, 1596600, 1596603

Signed-off-by: <bodonnel@redhat.com>

[PATCH 1/4] mkfs.xfs: avoid potential overflowing expression in xfs_mkfs.c

[Bill O'Donnell]

Cast max_tx_bytes to uint64_t to avoid overflowing expression in
calc_concurrency_logblocks().

Coverity-id: 1596603

Signed-off-by: Bill O'Donnell <bodonnel@redhat.com>
---
 mkfs/xfs_mkfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index f4a9bf20..2f801dd4 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -3678,7 +3678,7 @@ calc_concurrency_logblocks(
 	 * without blocking for space.  Increase the figure by 50% so that
 	 * background threads can also run.
 	 */
-	log_bytes = max_tx_bytes * 3 * cli->log_concurrency / 2;
+	log_bytes = (uint64_t)max_tx_bytes * 3 * cli->log_concurrency / 2;
 	new_logblocks = min(XFS_MAX_LOG_BYTES >> cfg->blocklog,
 				log_bytes >> cfg->blocklog);
 
-- 
2.45.2

[PATCH 2/4] xfs_db: fix unitialized variable ifake->if_levels

[Bill O'Donnell]

Initialize if_levels to 0.

Coverity-id: 1596600, 1596597

Signed-off-by: Bill O'Donnell <bodonnel@redhat.com>
---
 db/bmap_inflate.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/db/bmap_inflate.c b/db/bmap_inflate.c
index 33b0c954..8232f486 100644
--- a/db/bmap_inflate.c
+++ b/db/bmap_inflate.c
@@ -351,6 +351,7 @@ build_new_datafork(
 	/* Set up staging for the new bmbt */
 	ifake.if_fork = kmem_cache_zalloc(xfs_ifork_cache, 0);
 	ifake.if_fork_size = xfs_inode_fork_size(ip, XFS_DATA_FORK);
+	ifake.if_levels = 0;
 	bmap_cur = libxfs_bmbt_stage_cursor(ip->i_mount, ip, &ifake);
 
 	/*
@@ -404,6 +405,7 @@ estimate_size(
 
 	ifake.if_fork = kmem_cache_zalloc(xfs_ifork_cache, 0);
 	ifake.if_fork_size = xfs_inode_fork_size(ip, XFS_DATA_FORK);
+	ifake.if_levels = 0;
 
 	bmap_cur = libxfs_bmbt_stage_cursor(ip->i_mount, ip, &ifake);
 	error = -libxfs_btree_bload_compute_geometry(bmap_cur, &bmap_bload,
-- 
2.45.2

[PATCH 3/4] xfs_fsr: correct type in fsrprintf() call

[Bill O'Donnell]

Use %ld instead of %d for howlong variable.

Coverity-id: 1596598

Signed-off-by: Bill O'Donnell <bodonnel@redhat.com>
---
 fsr/xfs_fsr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fsr/xfs_fsr.c b/fsr/xfs_fsr.c
index fdd37756..d204e3a4 100644
--- a/fsr/xfs_fsr.c
+++ b/fsr/xfs_fsr.c
@@ -426,7 +426,7 @@ fsrallfs(char *mtab, time_t howlong, char *leftofffile)
 	fsdesc_t *fsp;
 	struct stat sb, sb2;
 
-	fsrprintf("xfs_fsr -m %s -t %d -f %s ...\n", mtab, howlong, leftofffile);
+	fsrprintf("xfs_fsr -m %s -t %ld -f %s ...\n", mtab, howlong, leftofffile);
 
 	endtime = starttime + howlong;
 	fs = fsbase;
-- 
2.45.2

[PATCH 4/4] xfs_repair: correct type of variable global_msgs.interval to time_t

[Bill O'Donnell]

Use time_t instead of int for interval field.

Coverity-id: 1596599

Signed-off-by: Bill O'Donnell <bodonnel@redhat.com>
---
 repair/progress.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/repair/progress.c b/repair/progress.c
index 2ce36cef..15455a99 100644
--- a/repair/progress.c
+++ b/repair/progress.c
@@ -91,7 +91,7 @@ typedef struct msg_block_s {
 	uint64_t	*done;
 	uint64_t	*total;
 	int		count;
-	int		interval;
+	time_t		interval;
 } msg_block_t;
 static msg_block_t 	global_msgs;
 
-- 
2.45.2

Re: [PATCH 1/4] mkfs.xfs: avoid potential overflowing expression in xfs_mkfs.c

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

> +	log_bytes = (uint64_t)max_tx_bytes * 3 * cli->log_concurrency / 2;
>  	new_logblocks = min(XFS_MAX_LOG_BYTES >> cfg->blocklog,
>  				log_bytes >> cfg->blocklog);
>  
> -- 
> 2.45.2
> 
> 
---end quoted text---

Re: [PATCH 2/4] xfs_db: fix unitialized variable ifake->if_levels

[Christoph Hellwig]

On Thu, Jun 13, 2024 at 01:07:06PM -0500, Bill O'Donnell wrote:
> Initialize if_levels to 0.
> 
> Coverity-id: 1596600, 1596597
> 
> Signed-off-by: Bill O'Donnell <bodonnel@redhat.com>
> ---
>  db/bmap_inflate.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/db/bmap_inflate.c b/db/bmap_inflate.c
> index 33b0c954..8232f486 100644
> --- a/db/bmap_inflate.c
> +++ b/db/bmap_inflate.c
> @@ -351,6 +351,7 @@ build_new_datafork(
>  	/* Set up staging for the new bmbt */
>  	ifake.if_fork = kmem_cache_zalloc(xfs_ifork_cache, 0);
>  	ifake.if_fork_size = xfs_inode_fork_size(ip, XFS_DATA_FORK);
> +	ifake.if_levels = 0;
>  	bmap_cur = libxfs_bmbt_stage_cursor(ip->i_mount, ip, &ifake);
>  
>  	/*
> @@ -404,6 +405,7 @@ estimate_size(
>  
>  	ifake.if_fork = kmem_cache_zalloc(xfs_ifork_cache, 0);
>  	ifake.if_fork_size = xfs_inode_fork_size(ip, XFS_DATA_FORK);
> +	ifake.if_levels = 0;

Maybe initialize it at declaration time by doing:

	struct xbtree_ifakeroot		ifake = { };

to future-proof against adding more fields?

Re: [PATCH 3/4] xfs_fsr: correct type in fsrprintf() call

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 4/4] xfs_repair: correct type of variable global_msgs.interval to time_t

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 2/4] xfs_db: fix unitialized variable ifake->if_levels

[Bill O'Donnell]

On Thu, Jun 13, 2024 at 11:36:51AM -0700, Christoph Hellwig wrote:
> On Thu, Jun 13, 2024 at 01:07:06PM -0500, Bill O'Donnell wrote:
> > Initialize if_levels to 0.
> > 
> > Coverity-id: 1596600, 1596597
> > 
> > Signed-off-by: Bill O'Donnell <bodonnel@redhat.com>
> > ---
> >  db/bmap_inflate.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/db/bmap_inflate.c b/db/bmap_inflate.c
> > index 33b0c954..8232f486 100644
> > --- a/db/bmap_inflate.c
> > +++ b/db/bmap_inflate.c
> > @@ -351,6 +351,7 @@ build_new_datafork(
> >  	/* Set up staging for the new bmbt */
> >  	ifake.if_fork = kmem_cache_zalloc(xfs_ifork_cache, 0);
> >  	ifake.if_fork_size = xfs_inode_fork_size(ip, XFS_DATA_FORK);
> > +	ifake.if_levels = 0;
> >  	bmap_cur = libxfs_bmbt_stage_cursor(ip->i_mount, ip, &ifake);
> >  
> >  	/*
> > @@ -404,6 +405,7 @@ estimate_size(
> >  
> >  	ifake.if_fork = kmem_cache_zalloc(xfs_ifork_cache, 0);
> >  	ifake.if_fork_size = xfs_inode_fork_size(ip, XFS_DATA_FORK);
> > +	ifake.if_levels = 0;
> 
> Maybe initialize it at declaration time by doing:
> 
> 	struct xbtree_ifakeroot		ifake = { };
> 
> to future-proof against adding more fields?
> 
Makes sense, I'll send a new version.
Thanks-
Bill