Re: FYI: questionable xfsdump code

Brian Foster
2015-07-28T14:09:04Z

On Tue, Jul 28, 2015 at 08:33:33AM +0200, Marc Lehmann wrote:
> Hi!
>
> While causally browsing xfsdump code,I found this, in
> common/getdents.c:getdents_wrap (in xfsdump
>
> off64_t last_offset = -1;
>
> ...
>
> while ((char *)kdp < kbuf + retval) {
> ...
>
> if ((sizeof(dp->d_ino) != sizeof(kdp->d_ino))
> || (sizeof(dp->d_off) != sizeof(kdp->d_off))) {
> /* Overflow. If there was at least one entry
> before this one, return them without error,
> otherwise signal overflow. */
> if (last_offset != -1) {
> lseek64(fd, last_offset, SEEK_SET);
> return (char *)dp - buf;
> }
> errno = EOVERFLOW;
> return -1;
> }
>
> last_offset = d_off;
>
> ...
> }
>
> While not necessarily a bug, this comment is very confused - there is no
> way to reach the code inside the if with last_offset != -1, as the if
> condition is a compiletime constant.
>

It looks like this changed in:

b1d6979f remove ancient sys_getdents code paths

It used to look like this:

if ((sizeof (dp->d_ino) != sizeof (kdp->d_ino)
&& dp->d_ino != d_ino)
|| (sizeof (dp->d_off) != sizeof (kdp->d_off)
&& dp->d_off != d_off))
{
...
}

... which probably made more sense.

Brian

> This might be harmless dead code from some refactorisation gone wrong,
> or indicative of some bug due to some logic error. In any case, I just
> wanted to bring it to your attention.
>
> And as a side note, memcpy would be more efficient here, especially as it
> is called very often, (and especially so on irix :-):
>
> memmove(dp->d_name, kdp->d_name,
> old_reclen - offsetof(struct kernel_dirent64, d_name));
>
> --
> The choice of a Deliantra, the free code+content MORPG
> -----==- _GNU_ http://www.deliantra.net
> ----==-- _ generation
> ---==---(_)__ __ ____ __ Marc Lehmann
> --==---/ / _ \/ // /\ \/ / schmorp@schmorp.de
> -=====/_/_//_/\_,_/ /_/\_\
>
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com