FYI: questionable xfsdump code
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));Signature · 11 lines
The choice of a Deliantra, the free code+content MORPG
-----==- _GNU_ http://www.deliantra.net
----==-- _ generation
---==---(_)__ __ ____ __ Marc Lehmann
--==---/ / _ \/ // /\ \/ / [email protected]
-=====/_/_//_/\_,_/ /_/\_\
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfsQuoted 30 lines · Marc Lehmann
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; ... }
Quoted 4 lines
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
Quoted 4 lines
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.
Quoted 18 lines
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));Signature · 11 lines
The choice of a Deliantra, the free code+content MORPG -----==- _GNU_ http://www.deliantra.net ----==-- _ generation ---==---(_)__ __ ____ __ Marc Lehmann --==---/ / _ \/ // /\ \/ / [email protected] -=====/_/_//_/\_,_/ /_/\_\ _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs