* [Cluster-devel] Re: gfs2-utils: master - gfs2: fix build warnings spotted by paranoia cflags
[not found] <20090514032012.BF20C12022E@lists.fedorahosted.org>
@ 2009-05-14 7:17 ` Andrew Price
2009-05-14 7:44 ` Fabio M. Di Nitto
0 siblings, 1 reply; 2+ messages in thread
From: Andrew Price @ 2009-05-14 7:17 UTC (permalink / raw)
To: cluster-devel.redhat.com
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@redhat.com>
> AuthorDate: Thu May 14 05:10:53 2009 +0200
> Committer: Fabio M. Di Nitto <fdinitto@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
^ permalink raw reply [flat|nested] 2+ messages in thread
* [Cluster-devel] Re: gfs2-utils: master - gfs2: fix build warnings spotted by paranoia cflags
2009-05-14 7:17 ` [Cluster-devel] Re: gfs2-utils: master - gfs2: fix build warnings spotted by paranoia cflags Andrew Price
@ 2009-05-14 7:44 ` Fabio M. Di Nitto
0 siblings, 0 replies; 2+ messages in thread
From: Fabio M. Di Nitto @ 2009-05-14 7:44 UTC (permalink / raw)
To: cluster-devel.redhat.com
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@redhat.com>
> > AuthorDate: Thu May 14 05:10:53 2009 +0200
> > Committer: Fabio M. Di Nitto <fdinitto@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
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2009-05-14 7:44 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20090514032012.BF20C12022E@lists.fedorahosted.org>
2009-05-14 7:17 ` [Cluster-devel] Re: gfs2-utils: master - gfs2: fix build warnings spotted by paranoia cflags Andrew Price
2009-05-14 7:44 ` Fabio M. Di Nitto
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.