ocfs2: Cleanup dirent size check
PATCH 10/15 ocfs2: Cleanup dirent size check
The check to see if a new dirent would fit in an old one is pretty ugly, and it's done at least twice. Clean things up by putting this in it's own easier-to-read function.
--- fs/ocfs2/dir.c | 36 ++++++++++++++++++++++++++++-------- 1 files changed, 28 insertions(+), 8 deletions(-)
fs/ocfs2/dir.c +28 -8
@@ -343,6 +343,31 @@ bail:return status; } +/* +* Check whether 'de' has enough room to hold an entry of +* 'new_rec_len' bytes. +*/ +static inline int ocfs2_dirent_would_fit (struct ocfs2_dir_entry *de, +unsigned int new_rec_len) +{ +unsigned int de_really_needed; + +/* Check whether this is an empty record with enough space */ +if (le64_to_cpu (de->inode) ==0 && +le16_to_cpu (de->rec_len) >= new_rec_len) +return 1 ; + +/* +* Record might have free space at the end which we can +* use. +*/ + de_really_needed =OCFS2_DIR_REC_LEN (de->name_len); +if (le16_to_cpu (de->rec_len) >= (de_really_needed + new_rec_len)) +return 1 ; + +return 0 ; +} +/* we don't always have a dentry for what we want to add, so people * like orphan dir can call this instead. *
@@ -385,10 +410,8 @@ int __ocfs2_add_entry(handle_t *handle, retval = -EEXIST ; goto bail; } -if (((le64_to_cpu (de->inode) ==0 ) && - (le16_to_cpu (de->rec_len) >= rec_len)) || - (le16_to_cpu (de->rec_len) >= - (OCFS2_DIR_REC_LEN (de->name_len) + rec_len))) { + +if (ocfs2_dirent_would_fit (de, rec_len)) { dir->i_mtime = dir->i_ctime =CURRENT_TIME ; retval =ocfs2_mark_inode_dirty (handle, dir, parent_fe_bh);if (retval <0 ) {
@@ -1078,10 +1101,7 @@ int ocfs2_prepare_dir_for_insert(struct ocfs2_super *osb, status = -EEXIST ; goto bail; } -if (((le64_to_cpu (de->inode) ==0 ) && - (le16_to_cpu (de->rec_len) >= rec_len)) || - (le16_to_cpu (de->rec_len) >= - (OCFS2_DIR_REC_LEN (de->name_len) + rec_len))) { +if (ocfs2_dirent_would_fit (de, rec_len)) {/* Ok, we found a spot. Return this bh and let * the caller actually fill it in. */ *ret_de_bh = bh;
Signature · 1 line
1.5.0.6
PATCH 10/15 ocfs2: Cleanup dirent size check
Quoted 5 lines · Mark Fasheh
The check to see if a new dirent would fit in an old one is pretty ugly, and it's done at least twice. Clean things up by putting this in it's own easier-to-read function.
Signed-off-by Mark Fasheh <[email protected]>
de_really_needed sounds like it's the new needed, which confused me (de_really_needed + new_rec_len sounds like "new needed + new needed", which of course makes no sense). Can you rename de_really_needed to de_really_used, de_old_used, or de_old_needed? That captures the sense of "how much space in the old dirent is actually used".
Quoted 73 lines
--- fs/ocfs2/dir.c | 36 ++++++++++++++++++++++++++++-------- 1 files changed, 28 insertions(+), 8 deletions(-)fs/ocfs2/dir.c +28
-8@@ -343,6 +343,31 @@ bail:return status; } +/* +* Check whether 'de' has enough room to hold an entry of +* 'new_rec_len' bytes. +*/ +static inline int ocfs2_dirent_would_fit (struct ocfs2_dir_entry *de, +unsigned int new_rec_len) +{ +unsigned int de_really_needed; + +/* Check whether this is an empty record with enough space */ +if (le64_to_cpu (de->inode) ==0 && +le16_to_cpu (de->rec_len) >= new_rec_len) +return 1 ; + +/* +* Record might have free space at the end which we can +* use. +*/ + de_really_needed =OCFS2_DIR_REC_LEN (de->name_len); +if (le16_to_cpu (de->rec_len) >= (de_really_needed + new_rec_len)) +return 1 ; + +return 0 ; +} +/* we don't always have a dentry for what we want to add, so people * like orphan dir can call this instead. * @@ -385,10 +410,8 @@ int __ocfs2_add_entry(handle_t *handle, retval = -EEXIST ; goto bail; } -if (((le64_to_cpu (de->inode) ==0 ) && - (le16_to_cpu (de->rec_len) >= rec_len)) || - (le16_to_cpu (de->rec_len) >= - (OCFS2_DIR_REC_LEN (de->name_len) + rec_len))) { + +if (ocfs2_dirent_would_fit (de, rec_len)) { dir->i_mtime = dir->i_ctime =CURRENT_TIME ; retval =ocfs2_mark_inode_dirty (handle, dir, parent_fe_bh);if (retval <0 ) {@@ -1078,10 +1101,7 @@ int ocfs2_prepare_dir_for_insert(struct ocfs2_super *osb, status = -EEXIST ; goto bail; } -if (((le64_to_cpu (de->inode) ==0 ) && - (le16_to_cpu (de->rec_len) >= rec_len)) || - (le16_to_cpu (de->rec_len) >= - (OCFS2_DIR_REC_LEN (de->name_len) + rec_len))) { +if (ocfs2_dirent_would_fit (de, rec_len)) {/* Ok, we found a spot. Return this bh and let * the caller actually fill it in. */ *ret_de_bh = bh;Signature · 7 lines
1.5.0.6 _______________________________________________ Ocfs2-devel mailing list [email protected] http://oss.oracle.com/mailman/listinfo/ocfs2-devel
Signature · 9 lines
"You can get more with a kind word and a gun than you can with
a kind word alone."
- Al Capone
Joel Becker
Principal Software Developer
Oracle
E-mail: [email protected]
Phone: (650) 506-8127PATCH 10/15 ocfs2: Cleanup dirent size check
Quoted 5 lines · Joel Becker
Quoted 4 lines · Mark Fasheh
The check to see if a new dirent would fit in an old one is pretty ugly, and it's done at least twice. Clean things up by putting this in it's own easier-to-read function.
Quoted 7 lines
Quoted 1 line
Signed-off-by Mark Fasheh <[email protected]>
de_really_neededsounds like it's the new needed, which confused me (de_really_needed+new_rec_lensounds like "new needed + new needed", which of course makes no sense). Can you renamede_really_neededtode_really_used,de_old_used, orde_old_needed? That captures the sense of "how much space in the old dirent is actually used".
Yeah, that variable naming even confuses me when I look at it :(
I'm going to spin the wheel of variable names and land at.... de_really_used!
--Mark
--
Mark Fasheh
Senior Software Developer, Oracle
[email protected]