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

[Fabio M. Di Nitto]

On Thu, 2009-05-14 at 08:17 +0100, Andrew Price wrote:
> 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.

In chain:
libgfs2.h be16_to_cpu(x) -> bswap16(x)
byteswap.h bswap16(x) -> __bswap_16(x)
bits/byteswap.h __bswap_16(x) -> does conversion in place and return the
swabbed value. So either way new->de_rec_len is converted to the right
endianess.


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

Same reason.. unless I am totally wrong.


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

No, this was a leftover while I was cleaning the tree because it was
triggering a warning in every single file that included libgfs2.h.

I am fixing this right away.

Fabio