xfs
[Top] [All Lists]

Re: [PATCH 2/2] xfsdump: don't do pointer math twice

To: rjohnston@xxxxxxx, xfs@xxxxxxxxxxx
Subject: Re: [PATCH 2/2] xfsdump: don't do pointer math twice
From: Eric Sandeen <sandeen@xxxxxxxxxxx>
Date: Fri, 21 Aug 2015 12:24:54 -0500
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20150821193241.973348815@xxxxxxxxxxxxxxxxxxxxxxx>
References: <20150821193047.661578219@xxxxxxxxxxxxxxxxxxxxxxx> <20150821193241.973348815@xxxxxxxxxxxxxxxxxxxxxxx>
On 8/21/15 9:01 AM, rjohnston@xxxxxxx wrote:
> The pointer math when calculating the address for the call to memset
> is incorrect, so we are clearing the wrong memory location.
> 
> i2gmap is of type i2gseg_t 
> 
> oldsize has already computed the pointer offset
>       oldsize = inomap.hnkmaplen * SEGPERHNK * sizeof(i2gseg_t);
> the memset call is using
>       inomap.i2gmap + oldsize == &inomap.i2gmap[oldsize]
> so we were doing the pointer math twice.
> 
> We already compensate for the length of each item in oldsize so
> adding need to add a (char *) cast to the memset parameter.


What about just doing:

                        memset(&inomap.i2gmap[numsegs - SEGPERHNK], 0,
                               SEGPERHNK * sizeof(i2gseg_t));

which more clearly shows that we're setting the new array members
to zero.

(could do oldsegs = inomap.hnkmaplen * SEGPERHNK; prior to the
hnkmaplen++, if that makes it any more readable...)

*shrug* it seems a little clearer to me, anyway.

Thanks,
-Eric

> ---
>  dump/inomap.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Index: b/dump/inomap.c
> ===================================================================
> --- a/dump/inomap.c
> +++ b/dump/inomap.c
> @@ -1143,7 +1143,7 @@ cb_add_inogrp( void *arg1, intgen_t fsfd
>                               return -1;
>  
>                       /* zero the new portion of the i2gmap */
> -                     memset(inomap.i2gmap + oldsize,
> +                     memset((char *)inomap.i2gmap + oldsize,
>                              0,
>                              SEGPERHNK * sizeof(i2gseg_t));
>               }
> 
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs
> 

<Prev in Thread] Current Thread [Next in Thread>