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 <[email protected]> > > AuthorDate: Thu May 14 05:10:53 2009 +0200 > > Committer: Fabio M. Di Nitto <[email protected]> > > 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
