[Cluster-devel] Re: gfs2-utils: master - gfs2: fix build warnings spotted by paranoia cflags

Andrew Price andy at andrewprice.me.uk
Thu May 14 07:17:38 UTC 2009


Hi,

Just a few remarks on this commit...

On Thu, May 14, 2009 at 03:20:12AM +0000, Fabio M. Di Nitto wrote:
> Gitweb:        http://git.fedorahosted.org/git/gfs2-utils.git?p=gfs2-utils.git;a=commitdiff;h=eca38b24f3066db8adfb648dfc16d221faaeee9c
> Commit:        eca38b24f3066db8adfb648dfc16d221faaeee9c
> Parent:        d021fa9f856976abcbc3a1fd3b77b7607bcad39d
> Author:        Fabio M. Di Nitto <fdinitto at redhat.com>
> AuthorDate:    Thu May 14 05:10:53 2009 +0200
> Committer:     Fabio M. Di Nitto <fdinitto at redhat.com>
> CommitterDate: Thu May 14 05:19:53 2009 +0200
> 
> gfs2: fix build warnings spotted by paranoia cflags
> 

> diff --git a/gfs2/libgfs2/fs_ops.c b/gfs2/libgfs2/fs_ops.c
> index a45144b..633901a 100644
> --- a/gfs2/libgfs2/fs_ops.c
> +++ b/gfs2/libgfs2/fs_ops.c
> @@ -43,9 +43,9 @@ struct gfs2_inode *inode_get(struct gfs2_sbd *sdp, struct gfs2_buffer_head *bh)
>  	return ip;
>  }
>  
> -void inode_put(struct gfs2_inode *ip, enum update_flags updated)
> +void inode_put(struct gfs2_inode *ip, enum update_flags is_updated)
>  {
> -	if (updated)
> +	if (is_updated)
>  		gfs2_dinode_out(&ip->i_di, ip->i_bh->b_data);
>  	brelse(ip->i_bh, updated);
>  	free(ip);
> @@ -681,8 +681,10 @@ static int dirent_alloc(struct gfs2_inode *dip, struct gfs2_buffer_head *bh,
>  				new->de_rec_len = cpu_to_be16(cur_rec_len -
>  											  GFS2_DIRENT_SIZE(cur_name_len));
>  				new->de_name_len = cpu_to_be16(name_len);
> -				dent->de_rec_len = cpu_to_be16(cur_rec_len -
> -											   be16_to_cpu(new->de_rec_len));
> +
> +				be16_to_cpu(new->de_rec_len);
> +				dent->de_rec_len = cpu_to_be16(cur_rec_len - new->de_rec_len);
> +

This doesn't look right to me. The be16_to_cpu(new->de_rec_len); doesn't
do anything on its own.

>  				*dent_out = new;
>  				return 0;
>  			}
> @@ -714,14 +716,14 @@ void dirent2_del(struct gfs2_inode *dip, struct gfs2_buffer_head *bh,
>  	prev->de_rec_len = cpu_to_be16(prev_rec_len);
>  }
>  
> -void gfs2_get_leaf_nr(struct gfs2_inode *dip, uint32_t index,
> +void gfs2_get_leaf_nr(struct gfs2_inode *dip, uint32_t lindex,
>  					  uint64_t *leaf_out)
>  {
>  	uint64_t leaf_no;
>  	int count;
>  
>  	count = gfs2_readi(dip, (char *)&leaf_no,
> -		      index * sizeof(uint64_t),
> +		      lindex * sizeof(uint64_t),
>  		      sizeof(uint64_t));
>  	if (count != sizeof(uint64_t))
>  		die("gfs2_get_leaf_nr:  Bad internal read.\n");
> @@ -741,7 +743,7 @@ void gfs2_put_leaf_nr(struct gfs2_inode *dip, uint32_t inx, uint64_t leaf_out)
>  		die("gfs2_put_leaf_nr:  Bad internal write.\n");
>  }
>  
> -static void dir_split_leaf(struct gfs2_inode *dip, uint32_t index, uint64_t leaf_no)
> +static void dir_split_leaf(struct gfs2_inode *dip, uint32_t lindex, uint64_t leaf_no)
>  {
>  	struct gfs2_buffer_head *nbh, *obh;
>  	struct gfs2_leaf *nleaf, *oleaf;
> @@ -771,7 +773,7 @@ static void dir_split_leaf(struct gfs2_inode *dip, uint32_t index, uint64_t leaf
>  	len = 1 << (dip->i_di.di_depth - be16_to_cpu(oleaf->lf_depth));
>  	half_len = len >> 1;
>  
> -	start = (index & ~(len - 1));
> +	start = (lindex & ~(len - 1));
>  
>  	lp = calloc(1, half_len * sizeof(uint64_t));
>  	if (lp == NULL) {
> @@ -920,12 +922,12 @@ int gfs2_get_leaf(struct gfs2_inode *dip, uint64_t leaf_no,
>   * Returns: 0 on success, error code otherwise
>   */
>  
> -static int get_first_leaf(struct gfs2_inode *dip, uint32_t index,
> +static int get_first_leaf(struct gfs2_inode *dip, uint32_t lindex,
>  			  struct gfs2_buffer_head **bh_out)
>  {
>  	uint64_t leaf_no;
>  
> -	gfs2_get_leaf_nr(dip, index, &leaf_no);
> +	gfs2_get_leaf_nr(dip, lindex, &leaf_no);
>  	*bh_out = bread(&dip->i_sbd->buf_list, leaf_no);
>  	return 0;
>  }
> @@ -952,13 +954,13 @@ static int get_next_leaf(struct gfs2_inode *dip,struct gfs2_buffer_head *bh_in,
>  	return 0;
>  }
>  
> -static void dir_e_add(struct gfs2_inode *dip, char *filename, int len,
> +static void dir_e_add(struct gfs2_inode *dip, const char *filename, int len,
>  		      struct gfs2_inum *inum, unsigned int type)
>  {
>  	struct gfs2_buffer_head *bh, *nbh;
>  	struct gfs2_leaf *leaf, *nleaf;
>  	struct gfs2_dirent *dent;
> -	uint32_t index;
> +	uint32_t lindex;
>  	uint32_t hash;
>  	uint64_t leaf_no, bn;
>  
> @@ -966,11 +968,11 @@ static void dir_e_add(struct gfs2_inode *dip, char *filename, int len,
>  	hash = gfs2_disk_hash(filename, len);
>  	/* Have to kludge because (hash >> 32) gives hash for some reason. */
>  	if (dip->i_di.di_depth)
> -		index = hash >> (32 - dip->i_di.di_depth);
> +		lindex = hash >> (32 - dip->i_di.di_depth);
>  	else
> -		index = 0;
> +		lindex = 0;
>  
> -	gfs2_get_leaf_nr(dip, index, &leaf_no);
> +	gfs2_get_leaf_nr(dip, lindex, &leaf_no);
>  
>  	for (;;) {
>  		bh = bread(&dip->i_sbd->buf_list, leaf_no);
> @@ -980,7 +982,7 @@ static void dir_e_add(struct gfs2_inode *dip, char *filename, int len,
>  
>  			if (be16_to_cpu(leaf->lf_depth) < dip->i_di.di_depth) {
>  				brelse(bh, not_updated);
> -				dir_split_leaf(dip, index, leaf_no);
> +				dir_split_leaf(dip, lindex, leaf_no);
>  				goto restart;
>  
>  			} else if (dip->i_di.di_depth < GFS2_DIR_MAX_DEPTH) {
> @@ -1071,7 +1073,8 @@ static void dir_make_exhash(struct gfs2_inode *dip)
>  			break;
>  	} while (gfs2_dirent_next(dip, bh, &dent) == 0);
>  
> -	dent->de_rec_len = cpu_to_be16(be16_to_cpu(dent->de_rec_len) +
> +	be16_to_cpu(dent->de_rec_len);
> +	dent->de_rec_len = cpu_to_be16(dent->de_rec_len +

Same here.

>  		sizeof(struct gfs2_dinode) - sizeof(struct gfs2_leaf));
>  
>  	brelse(bh, updated);
> @@ -1092,7 +1095,7 @@ static void dir_make_exhash(struct gfs2_inode *dip)
>  	dip->i_di.di_depth = y;
>  }
>  
> -static void dir_l_add(struct gfs2_inode *dip, char *filename, int len,
> +static void dir_l_add(struct gfs2_inode *dip, const char *filename, int len,
>  		      struct gfs2_inum *inum, unsigned int type)
>  {
>  	struct gfs2_dirent *dent;
> @@ -1112,7 +1115,7 @@ static void dir_l_add(struct gfs2_inode *dip, char *filename, int len,
>  	dip->i_di.di_entries++;
>  }
>  
> -void dir_add(struct gfs2_inode *dip, char *filename, int len,
> +void dir_add(struct gfs2_inode *dip, const char *filename, int len,
>  	     struct gfs2_inum *inum, unsigned int type)
>  {
>  	if (dip->i_di.di_flags & GFS2_DIF_EXHASH)
> @@ -1183,7 +1186,7 @@ struct gfs2_buffer_head *init_dinode(struct gfs2_sbd *sdp,
>  	return bh;
>  }
>  
> -struct gfs2_inode *createi(struct gfs2_inode *dip, char *filename,
> +struct gfs2_inode *createi(struct gfs2_inode *dip, const char *filename,
>  			   unsigned int mode, uint32_t flags)
>  {
>  	struct gfs2_sbd *sdp = dip->i_sbd;
> @@ -1303,7 +1306,7 @@ static int linked_leaf_search(struct gfs2_inode *dip, const char *filename,
>  			      struct gfs2_buffer_head **bh_out)
>  {
>  	struct gfs2_buffer_head *bh = NULL, *bh_next;
> -	uint32_t hsize, index;
> +	uint32_t hsize, lindex;
>  	uint32_t hash;
>  	int error = 0;
>  
> @@ -1314,9 +1317,9 @@ static int linked_leaf_search(struct gfs2_inode *dip, const char *filename,
>  	/*  Figure out the address of the leaf node.  */
>  
>  	hash = gfs2_disk_hash(filename, len);
> -	index = hash >> (32 - dip->i_di.di_depth);
> +	lindex = hash >> (32 - dip->i_di.di_depth);
>  
> -	error = get_first_leaf(dip, index, &bh_next);
> +	error = get_first_leaf(dip, lindex, &bh_next);
>  	if (error)
>  		return error;
>  
> @@ -1438,17 +1441,17 @@ static int dir_search(struct gfs2_inode *dip, const char *filename, int len,
>  
>  static int dir_e_del(struct gfs2_inode *dip, const char *filename, int len)
>  {
> -	int index;
> +	int lindex;
>  	int error;
>  	int found = 0;
>  	uint64_t leaf_no;
>  	struct gfs2_buffer_head *bh = NULL;
>  	struct gfs2_dirent *cur, *prev;
>  
> -	index = (1 << (dip->i_di.di_depth))-1;
> +	lindex = (1 << (dip->i_di.di_depth))-1;
>  
> -	for(; (index >= 0) && !found; index--){
> -		gfs2_get_leaf_nr(dip, index, &leaf_no);
> +	for(; (lindex >= 0) && !found; lindex--){
> +		gfs2_get_leaf_nr(dip, lindex, &leaf_no);
>  
>  		while(leaf_no && !found){
>  			bh = bread(&dip->i_sbd->buf_list, leaf_no);


> diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h
> index 5022f75..5eef1a6 100644
> --- a/gfs2/libgfs2/libgfs2.h
> +++ b/gfs2/libgfs2/libgfs2.h
> @@ -74,7 +74,7 @@ static __inline__ __attribute__((noreturn)) void die(const char *fmt, ...)
>  	va_list ap;
>  	fprintf(stderr, "%s: ", __FILE__);
>  	va_start(ap, fmt);
> -	vfprintf(stderr, fmt, ap);
> +	//vfprintf(stderr, fmt, ap);

Is this needed?

The rest looks ok to me.

--
Andrew Price




More information about the Cluster-devel mailing list