All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marc Lehmann <schmorp@schmorp.de>
To: xfs@oss.sgi.com
Subject: FYI: questionable xfsdump code
Date: Tue, 28 Jul 2015 08:33:33 +0200	[thread overview]
Message-ID: <20150728063246.GA2510@schmorp.de> (raw)

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@schmorp.de
      -=====/_/_//_/\_,_/ /_/\_\

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

             reply	other threads:[~2015-07-28  6:33 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-28  6:33 Marc Lehmann [this message]
2015-07-28 14:08 ` FYI: questionable xfsdump code Brian Foster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150728063246.GA2510@schmorp.de \
    --to=schmorp@schmorp.de \
    --cc=xfs@oss.sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.