Some code review findings for nilfs-utils

Some code review findings for nilfs-utils

[Eric Sandeen]

We ran an automated code checker over nilfs-utils-2.0.13, and it found
just a few issues:

                     start < NILFS_CNO_MIN || end > NILFS_CNO_MAX) {
 /BUILD/nilfs-utils-2.0.13/bin/rmcp.c  203  Warning 685:
     Relational operator '>' always evaluates to 'false'


                         oflags |= O_RDONLY;
 /BUILD/nilfs-utils-2.0.13/lib/nilfs.c  294  Info 835: A
     zero has been given as right argument to operator '|'


 /BUILD/nilfs-utils-2.0.13/lib/nilfs.c  899  Warning 438:
     Last value assigned to variable 'n' (defined at line 895) not used


         pseg->p_maxblocks = nblocks_per_segment - blkoff;
 /BUILD/nilfs-utils-2.0.13/lib/nilfs.c  732  Info 712: Loss
     of precision (assignment) (unsigned long to unsigned int)


Sorry for not sending patches, but I assume the maintainers can do a
proper fix more quickly than I can.  :)

Thanks,

-Eric

Re: Some code review findings for nilfs-utils

[Ryusuke Konishi]

Hi,
On Fri, 10 Jul 2009 11:13:00 -0500, Eric Sandeen wrote:
> We ran an automated code checker over nilfs-utils-2.0.13, and it found
> just a few issues:
> 
>                      start < NILFS_CNO_MIN || end > NILFS_CNO_MAX) {
>  /BUILD/nilfs-utils-2.0.13/bin/rmcp.c  203  Warning 685:
>      Relational operator '>' always evaluates to 'false'
> 
> 
>                          oflags |= O_RDONLY;
>  /BUILD/nilfs-utils-2.0.13/lib/nilfs.c  294  Info 835: A
>      zero has been given as right argument to operator '|'
> 
> 
>  /BUILD/nilfs-utils-2.0.13/lib/nilfs.c  899  Warning 438:
>      Last value assigned to variable 'n' (defined at line 895) not used
> 
> 
>          pseg->p_maxblocks = nblocks_per_segment - blkoff;
>  /BUILD/nilfs-utils-2.0.13/lib/nilfs.c  732  Info 712: Loss
>      of precision (assignment) (unsigned long to unsigned int)
> 
> 
> Sorry for not sending patches, but I assume the maintainers can do a
> proper fix more quickly than I can.  :)
> 
> Thanks,
> 
> -Eric

Thanks.  I picked up the first and the third warnings and have pushed
the fixes into the git repo.  ;)

The rest were ok.  The following patch is a digest of the two.

Cheers,
Ryusuke Konishi
--
diff --git a/bin/rmcp.c b/bin/rmcp.c
index 93e0511..2d7a2b9 100644
--- a/bin/rmcp.c
+++ b/bin/rmcp.c
@@ -199,8 +199,7 @@ int main(int argc, char *argv[])
 	for ( ; optind < argc; optind++) {
 		if (nilfs_parse_cno_range(argv[optind], &start, &end,
 					  RMCP_BASE) < 0 ||
-		    start > end ||
-		    start < NILFS_CNO_MIN || end > NILFS_CNO_MAX) {
+		    start > end || start < NILFS_CNO_MIN) {
 			fprintf(stderr,
 				"%s: invalid checkpoint range: %s\n",
 				progname, argv[optind]);
diff --git a/lib/nilfs.c b/lib/nilfs.c
index df8792b..63570f4 100644
--- a/lib/nilfs.c
+++ b/lib/nilfs.c
@@ -913,9 +913,7 @@ void nilfs_block_next(struct nilfs_block *blk)
 nilfs_cno_t nilfs_get_oldest_cno(struct nilfs *nilfs)
 {
 	struct nilfs_cpinfo cpinfo[1];
-	ssize_t n;
 	
-	n = nilfs_get_cpinfo(nilfs, nilfs->n_mincno, NILFS_CHECKPOINT,
-			     cpinfo, 1);
+	nilfs_get_cpinfo(nilfs, nilfs->n_mincno, NILFS_CHECKPOINT, cpinfo, 1);
 	return nilfs->n_mincno;
 }