Bug 142924 - [ext2fs] [patch] Small cleanup for the inode struct in ext2fs (based on UFS)
Summary: [ext2fs] [patch] Small cleanup for the inode struct in ext2fs (based on UFS)
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: Unspecified
Hardware: Any Any
: Normal Affects Only Me
Assignee: freebsd-fs (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-01-18 02:50 UTC by Pedro F. Giffuni
Modified: 2011-01-02 11:03 UTC (History)
0 users

See Also:


Attachments
file.diff (1.83 KB, patch)
2010-01-18 02:50 UTC, Pedro F. Giffuni
no flags Details | Diff
patch-ext2fs (2.37 KB, application/octet-stream)
2010-01-24 23:20 UTC, Pedro F. Giffuni
no flags Details
patch-ext2fs (4.13 KB, application/octet-stream)
2010-01-28 18:22 UTC, Pedro F. Giffuni
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Pedro F. Giffuni 2010-01-18 02:50:05 UTC
Minor enhancements based on UFS:

From SVN 118969:
Eliminate the i_devvp field from the incore EXT2FS inodes, we can
get the same value from ip->i_ump->um_devvp.

This saves a pointer in the memory copies of inodes, which can
easily run into several hundred kilobytes.

The extra indirection is unmeasurable in benchmarks.
____

While here move a line before a comment in the lookup code to
make space for a dirhash implementation.
Comment 1 Mark Linimon freebsd_committer freebsd_triage 2010-01-18 02:52:09 UTC
Responsible Changed
From-To: freebsd-bugs->freebsd-fs

Over to maintainer(s).
Comment 2 Pedro F. Giffuni 2010-01-24 23:20:12 UTC
I added the equivalent to ufs_lookup.c 1.34:
Bug fixes for currently harmless bugs that could rise to bite
the unwary if the code were called in slightly different ways.

- In ufs_lookup() there is an off-by-one error in the test that checks
if dp->i_diroff is outside the range of the the current directory size.
This is completely harmless, since the following while-loop condition
'dp->i_offset < endsearch' is never met, so the code immediately
does a second pass starting at dp->i_offset = 0.

- Again in ufs_lookup(), the condition in a sanity check is wrong
for directories that are longer than one block. This bug means that



      
Comment 3 Pedro F. Giffuni 2010-01-28 18:22:25 UTC
Include two more simple patches from ufs_lookup.c:

CVS 1.54:
When compacting directories, ufs_direnter() always trusted DIRSIZ()
to supply the number of bytes to be bcopy()'d to move an entry. If
d_ino == 0 however, DIRSIZ() is not guaranteed to return a sensible
length, so ufs_direnter could end up corrupting a directory during
compaction.

CVS 1.45:
Extend the sanity checks in ufs_lookup to ensure that each directory
entry fits within its DIRBLKSIZ block.
_______

These were meant to fix issues found with dirhash on UFS but ext2fs
still works here with those changes so I think it's good to have
them, JIC we end up bringing over dirhash to ext2fs.


      
Comment 4 Mark Linimon 2010-01-28 18:37:32 UTC
----- Forwarded message from "Pedro F. Giffuni" <giffunip@tutopia.com> -----

From: "Pedro F. Giffuni" <giffunip@tutopia.com>
To: FreeBSD-gnats-submit@FreeBSD.org, freebsd-bugs@FreeBSD.org
Subject: Re: kern/142924: Small cleanup for the inode struct in ext2fs
	(based on UFS)

Include two more simple patches from ufs_lookup.c:

CVS 1.54:
When compacting directories, ufs_direnter() always trusted DIRSIZ()
to supply the number of bytes to be bcopy()'d to move an entry. If
d_ino == 0 however, DIRSIZ() is not guaranteed to return a sensible
length, so ufs_direnter could end up corrupting a directory during
compaction.

CVS 1.45:
Extend the sanity checks in ufs_lookup to ensure that each directory
entry fits within its DIRBLKSIZ block.
_______

These were meant to fix issues found with dirhash on UFS but ext2fs
still works here with those changes so I think it's good to have
them, JIC we end up bringing over dirhash to ext2fs.

----- End forwarded message -----
Comment 5 Pedro F. Giffuni 2010-12-31 16:05:23 UTC
Please close: this has been superceeded by PR 153584.
Comment 6 Jaakko Heinonen freebsd_committer freebsd_triage 2011-01-02 11:03:27 UTC
State Changed
From-To: open->closed

Closed at submitter's request. Superseded by kern/153584.