xfs
[Top] [All Lists]

Re: xfstests, weird looking code in src/resvtest.c

To: scameron@xxxxxxxxxxxxxxxxxx
Subject: Re: xfstests, weird looking code in src/resvtest.c
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Wed, 18 Jun 2014 09:06:27 +1000
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20140617162044.GL29459@xxxxxxxxxxxxxxxxxx>
References: <20140617162044.GL29459@xxxxxxxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Tue, Jun 17, 2014 at 11:20:44AM -0500, scameron@xxxxxxxxxxxxxxxxxx wrote:
> 
> This code in xfstests src/resvtest.c looks pretty strange:
> 
> ...
>  32         char            *readbuffer, *writebuffer;
> ...
>  70         readbuffer = memalign(psize, bsize);
>  71         writebuffer = memalign(psize, bsize);
>  72         if (!readbuffer || !writebuffer) {
>  73                 perror("open");
>  74                 exit(1);
>  75         }
>  76         memset(writebuffer, 'A', sizeof(writebuffer));
> 
> ^^^ writebuffer is a pointer, so using sizeof(writebuffer) here is
> odd. Is it intentional to put either 4 or 8 A's into writebuffer
> depending on sizeof a pointer?  Seems unlikely.
> 
> 110         while (++n < iterations) {
> 111                 char *p;
> 112                 int numerrors;
> 113 
> 114                 if (write(writefd, writebuffer, sizeof(writebuffer)) < 0) 
> {
> 115                         perror("write");
> 116                         exit(1);
> 117                 }
> 
> So that write will write sizeof a pointer's worth of whatever's in 
> writebuffer.
> Intentional?  Again, seems unlikely.
> 
> This seems like maybe somebody initially declared writebuffer as an array, but
> later went back and changed it to a pointer, but forgot to fixup everywhere 
> that
> referred to sizeof(writebuffer).
> 
> I would have sent a patch but I'm not sure what this code is trying to do.
> 
> gcc 4.4.7 (what comes with RHEL6u5) doesn't warn about this, but 4.8.3 does.

There's a recent patch on the fstests list (fstests@xxxxxxxxxxxxxxx)
that fixes this that I haven't picked up yet.

For actual test harness issues, you should use
fstests@xxxxxxxxxxxxxxx now, not xfs@xxxxxxxxxxxxxx

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

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