* duplicate code in dir2
@ 2011-06-22 15:24 Eric Sandeen
2011-06-22 15:25 ` Eric Sandeen
2011-06-23 10:53 ` Christoph Hellwig
0 siblings, 2 replies; 4+ messages in thread
From: Eric Sandeen @ 2011-06-22 15:24 UTC (permalink / raw)
To: xfs-oss
I was poking around with various code metrics, and found a fair bit of duplication in dir2 code (using "duplo").
Haven't really thought about how it might be factorable, but thought it might be interesting to share.
-Eric
/src/git/linux-2.6/fs/xfs/xfs_dir2_node.c(251)
/src/git/linux-2.6/fs/xfs/xfs_dir2_leaf.c(440)
if (index < be16_to_cpu(leaf->hdr.count))
memmove(lep + 1, lep,
(be16_to_cpu(leaf->hdr.count) - index) * sizeof(*lep));
lfloglow = index;
lfloghigh = be16_to_cpu(leaf->hdr.count);
be16_add_cpu(&leaf->hdr.count, 1);
else {
if (compact == 0) {
for (lowstale = index - 1;
lowstale >= 0 &&
be32_to_cpu(leaf->ents[lowstale].address) !=
XFS_DIR2_NULL_DATAPTR;
lowstale--)
continue;
for (highstale = index;
highstale < be16_to_cpu(leaf->hdr.count) &&
be32_to_cpu(leaf->ents[highstale].address) !=
XFS_DIR2_NULL_DATAPTR &&
(lowstale < 0 ||
index - lowstale - 1 >= highstale - index);
highstale++)
continue;
if (lowstale >= 0 &&
(highstale == be16_to_cpu(leaf->hdr.count) ||
index - lowstale - 1 < highstale - index)) {
/src/git/linux-2.6/fs/xfs/xfs_dir2_node.c(300)
/src/git/linux-2.6/fs/xfs/xfs_dir2_leaf.c(499)
if (index - lowstale - 1 > 0)
memmove(&leaf->ents[lowstale],
&leaf->ents[lowstale + 1],
(index - lowstale - 1) * sizeof(*lep));
lep = &leaf->ents[index - 1];
lfloglow = MIN(lowstale, lfloglow);
lfloghigh = MAX(index - 1, lfloghigh);
else {
/src/git/linux-2.6/fs/xfs/xfs_dir2_node.c(316)
/src/git/linux-2.6/fs/xfs/xfs_dir2_leaf.c(518)
if (highstale - index > 0)
memmove(&leaf->ents[index + 1],
&leaf->ents[index],
(highstale - index) * sizeof(*lep));
lep = &leaf->ents[index];
lfloglow = MIN(index, lfloglow);
lfloghigh = MAX(highstale, lfloghigh);
be16_add_cpu(&leaf->hdr.stale, -1);
lep->hashval = cpu_to_be32(args->hashval);
/src/git/linux-2.6/fs/xfs/xfs_dir2_node.c(582)
/src/git/linux-2.6/fs/xfs/xfs_dir2_leaf.c(1349)
for (lep = &leaf->ents[index]; index < be16_to_cpu(leaf->hdr.count) &&
be32_to_cpu(lep->hashval) == args->hashval;
lep++, index++) {
if (be32_to_cpu(lep->address) == XFS_DIR2_NULL_DATAPTR)
continue;
newdb = xfs_dir2_dataptr_to_db(mp, be32_to_cpu(lep->address));
if (newdb != curdb) {
/src/git/linux-2.6/fs/xfs/xfs_dir2_node.c(442)
/src/git/linux-2.6/fs/xfs/xfs_dir2_leaf.c(1349)
for (lep = &leaf->ents[index]; index < be16_to_cpu(leaf->hdr.count) &&
be32_to_cpu(lep->hashval) == args->hashval;
lep++, index++) {
if (be32_to_cpu(lep->address) == XFS_DIR2_NULL_DATAPTR)
continue;
newdb = xfs_dir2_dataptr_to_db(mp, be32_to_cpu(lep->address));
if (newdb != curdb) {
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: duplicate code in dir2
2011-06-22 15:24 duplicate code in dir2 Eric Sandeen
@ 2011-06-22 15:25 ` Eric Sandeen
2011-06-23 11:03 ` Christoph Hellwig
2011-06-23 10:53 ` Christoph Hellwig
1 sibling, 1 reply; 4+ messages in thread
From: Eric Sandeen @ 2011-06-22 15:25 UTC (permalink / raw)
To: xfs-oss
Another interesting chunk elsewhere:
/src/git/linux-2.6/fs/xfs/xfs_da_btree.c(782)
/src/git/linux-2.6/fs/xfs/xfs_attr_leaf.c(1542)
*action = 0;
return(0);
if (count == 0) {
forward = (info->forw != 0);
memcpy(&state->altpath, &state->path, sizeof(state->path));
error = xfs_da_path_shift(state, &state->altpath, forward,
0, &retval);
if (error)
return(error);
if (retval) {
*action = 0;
} else {
*action = 2;
return(0);
forward = (be32_to_cpu(info->forw) < be32_to_cpu(info->back));
for (i = 0; i < 2; forward = !forward, i++) {
if (forward)
blkno = be32_to_cpu(info->forw);
else
blkno = be32_to_cpu(info->back);
if (blkno == 0)
continue;
error = xfs_da_read_buf(state->args->trans, state->args->dp,
/src/git/linux-2.6/fs/xfs/xfs_da_btree.c(842)
/src/git/linux-2.6/fs/xfs/xfs_attr_leaf.c(1605)
break;
if (i >= 2) {
*action = 0;
return(0);
memcpy(&state->altpath, &state->path, sizeof(state->path));
if (blkno < blk->blkno) {
error = xfs_da_path_shift(state, &state->altpath, forward,
0, &retval);
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: duplicate code in dir2
2011-06-22 15:25 ` Eric Sandeen
@ 2011-06-23 11:03 ` Christoph Hellwig
0 siblings, 0 replies; 4+ messages in thread
From: Christoph Hellwig @ 2011-06-23 11:03 UTC (permalink / raw)
To: Eric Sandeen; +Cc: xfs-oss
On Wed, Jun 22, 2011 at 10:25:43AM -0500, Eric Sandeen wrote:
> Another interesting chunk elsewhere:
>
> /src/git/linux-2.6/fs/xfs/xfs_da_btree.c(782)
> /src/git/linux-2.6/fs/xfs/xfs_attr_leaf.c(1542)
> *action = 0;
> return(0);
> if (count == 0) {
The da_btree code is a big mess with things duplicated all over.
There's a third copy of this in the dir2 code as well. I have started
consolidating some of this code a while ago in prepation of adding
checksumming to the dir and attr code, but it has bit rotted a bit since
and I've not managed to get back to it.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: duplicate code in dir2
2011-06-22 15:24 duplicate code in dir2 Eric Sandeen
2011-06-22 15:25 ` Eric Sandeen
@ 2011-06-23 10:53 ` Christoph Hellwig
1 sibling, 0 replies; 4+ messages in thread
From: Christoph Hellwig @ 2011-06-23 10:53 UTC (permalink / raw)
To: Eric Sandeen; +Cc: xfs-oss
> /src/git/linux-2.6/fs/xfs/xfs_dir2_node.c(251)
> /src/git/linux-2.6/fs/xfs/xfs_dir2_leaf.c(440)
> if (index < be16_to_cpu(leaf->hdr.count))
> memmove(lep + 1, lep,
> (be16_to_cpu(leaf->hdr.count) - index) * sizeof(*lep));
> lfloglow = index;
> lfloghigh = be16_to_cpu(leaf->hdr.count);
> be16_add_cpu(&leaf->hdr.count, 1);
> else {
This one and the the chunks following it are easily factorable, and I've
created a patch. It's 100 lines of common code, just with the order of
asssers switched, and one of them reusing the lep pointer on entry, and
the other one recalculating it. With the function header and lots of
parameters we don't actually remove a whole lot of code, but having this
relatively complex piece of code only once sounds like a good idea.
> /src/git/linux-2.6/fs/xfs/xfs_dir2_node.c(582)
> /src/git/linux-2.6/fs/xfs/xfs_dir2_leaf.c(1349)
> for (lep = &leaf->ents[index]; index < be16_to_cpu(leaf->hdr.count) &&
> be32_to_cpu(lep->hashval) == args->hashval;
> lep++, index++) {
> if (be32_to_cpu(lep->address) == XFS_DIR2_NULL_DATAPTR)
> continue;
> newdb = xfs_dir2_dataptr_to_db(mp, be32_to_cpu(lep->address));
> if (newdb != curdb) {
>
> /src/git/linux-2.6/fs/xfs/xfs_dir2_node.c(442)
> /src/git/linux-2.6/fs/xfs/xfs_dir2_leaf.c(1349)
> for (lep = &leaf->ents[index]; index < be16_to_cpu(leaf->hdr.count) &&
> be32_to_cpu(lep->hashval) == args->hashval;
> lep++, index++) {
> if (be32_to_cpu(lep->address) == XFS_DIR2_NULL_DATAPTR)
> continue;
> newdb = xfs_dir2_dataptr_to_db(mp, be32_to_cpu(lep->address));
> if (newdb != curdb) {
This seems like a relatively common patter, which has even more
occurances with minimal variations, but I'm not sure there's an easy way
to factor it.
I'd prefer to rewrite the loops to something more readable like:
for (; index < be16_to_cpu(leaf->hdr.count); index++) {
lep = &leaf->ents[index];
if (be32_to_cpu(lep->hashval) != args->hashval)
break;
if (be32_to_cpu(lep->address) == XFS_DIR2_NULL_DATAPTR)
continue;
...
}
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-06-23 11:03 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-06-22 15:24 duplicate code in dir2 Eric Sandeen
2011-06-22 15:25 ` Eric Sandeen
2011-06-23 11:03 ` Christoph Hellwig
2011-06-23 10:53 ` Christoph Hellwig
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox