xfs
[Top] [All Lists]

FYI: questionable xfsdump code

To: xfs@xxxxxxxxxxx
Subject: FYI: questionable xfsdump code
From: Marc Lehmann <schmorp@xxxxxxxxxx>
Date: Tue, 28 Jul 2015 08:33:33 +0200
Delivered-to: xfs@xxxxxxxxxxx
Openpgp: id=904ad2f81fb16978e7536f726dea2ba30bc39eb6; url=http://pgp.schmorp.de/schmorp-pgpkey.txt; preference=signencrypt
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.

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@xxxxxxxxxx
      -=====/_/_//_/\_,_/ /_/\_\

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