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