Skip to contentSkip to conversation

Some code review findings for nilfs-utils

DISCUSSION Eric Sandeen 2 participants 2 messages Jul 10, 2009 1 message not in the archive

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

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

A message that is not in the archive (sent off-list or not yet received): [email protected]

#2 lore raw

Hi,

Quoted 22 lines · 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)

Quoted 6 lines

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
--

bin/rmcp.c +1 -2
index 93e0511..2d7a2b9 100644
@@ -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]);
lib/nilfs.c +1 -3
index df8792b..63570f4 100644
@@ -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;
 }