lkml.org 
[lkml]   [2005]   [Nov]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
Subject[RFC,PATCH] libfs dcache_readdir() and dcache_dir_lseek() bugfix
This patch effects all users of libfs' dcache directory implementation.

Old glibc implementations (e.g. glibc-2.2.5) are lseeking after every call to
getdents(), subsequent calls to getdents() are starting to read from a wrong
f_pos, when the directory is modified in between. Therefore not all directory
entries are returned. IMHO this is a bug and it breaks applications, e.g. "rm
-fr" on tmpfs.

SuSV3 only says:
"If a file is removed from or added to the directory after the most recent
call to opendir() or rewinddir(), whether a subsequent call to readdir_r()
returns an entry for that file is unspecified."

Although I tried to provide a small fix, this is a complete rework of the
libfs' dcache_readdir() and dcache_dir_lseek(). This patch use the dentries
name hashes as the offsets for the directory entries. I'm not that sure it is
maybe a problem to use full_name_hash() for that. If hash collisions occur,
readdir() might return duplicate entries after an lseek(). Any ideas?

This patch is compile and boot tested. This patch is against 2.6.14-git of
today.

Comments, please.
Jan

--
Jan Blunck jblunck@suse.de
SuSE LINUX AG - A Novell company
Maxfeldstr. 5 +49-911-74053-608
D-90409 Nürnberg http://www.suse.de
#include <stdio.h>
#include <unistd.h>
#include <dirent.h>

int main(int argc, char *argv[])
{
DIR *dir;
struct dirent *entry;
unsigned int offset;
if (argc < 2) {
printf("USAGE: %s <directory>\n", argv[0]);
return 1;
}
dir = opendir(argv[1]);
if (!dir)
return 1;
while((entry = readdir(dir)) != NULL) {
seekdir(dir, entry->d_off);
printf("name=%s\tino=%d off=%d\n", entry->d_name, entry->d_ino,
entry->d_off);
if (*entry->d_name == '.')
continue;
if(unlink(entry->d_name) != 0)
break;
}
closedir(dir);
return 0;
} fs/libfs.c | 140 +++++++++++++++++++++++++++++++++----------------------------
1 files changed, 76 insertions(+), 64 deletions(-)
Index: linux-2.6/fs/libfs.c
===================================================================
--- linux-2.6.orig/fs/libfs.c
+++ linux-2.6/fs/libfs.c
@@ -72,42 +72,57 @@ int dcache_dir_close(struct inode *inode
return 0;
}

-loff_t dcache_dir_lseek(struct file *file, loff_t offset, int origin)
+loff_t dcache_dir_lseek(struct file * file, loff_t offset, int origin)
{
+ struct dentry * dentry = file->f_dentry;
+ int found = 0;
+
+ /* for directories it doesn't make any sense to seek relative */
+ if (origin != 0)
+ return -EINVAL;
+
down(&file->f_dentry->d_inode->i_sem);
- switch (origin) {
- case 1:
- offset += file->f_pos;
- case 0:
- if (offset >= 0)
- break;
- default:
- up(&file->f_dentry->d_inode->i_sem);
- return -EINVAL;
- }
if (offset != file->f_pos) {
- file->f_pos = offset;
- if (file->f_pos >= 2) {
- struct list_head *p;
- struct dentry *cursor = file->private_data;
- loff_t n = file->f_pos - 2;
+ struct list_head *p;
+ struct dentry *cursor = file->private_data;

+ // Handle the special values
+ if ((offset == 0)
+ || (offset == full_name_hash(".", 1))
+ || (offset == full_name_hash("..", 2))) {
+ found = 1;
+ file->f_pos = offset;
spin_lock(&dcache_lock);
list_del(&cursor->d_child);
- p = file->f_dentry->d_subdirs.next;
- while (n && p != &file->f_dentry->d_subdirs) {
- struct dentry *next;
- next = list_entry(p, struct dentry, d_child);
- if (!d_unhashed(next) && next->d_inode)
- n--;
- p = p->next;
+ list_add(&cursor->d_child, &dentry->d_subdirs);
+ spin_unlock(&dcache_lock);
+ goto out;
+ }
+
+ spin_lock(&dcache_lock);
+ list_del(&cursor->d_child);
+ p = file->f_dentry->d_subdirs.next;
+ while (p != &file->f_dentry->d_subdirs) {
+ struct dentry *next;
+ next = list_entry(p, struct dentry, d_child);
+ if (!d_unhashed(next) && next->d_inode
+ && (offset == full_name_hash(next->d_name.name,
+ next->d_name.len))) {
+ found = 1;
+ break;
}
+ p = p->next;
+ }
+
+ if (found) {
list_add_tail(&cursor->d_child, p);
- spin_unlock(&dcache_lock);
+ file->f_pos = offset;
}
+ spin_unlock(&dcache_lock);
}
+out:
up(&file->f_dentry->d_inode->i_sem);
- return offset;
+ return (found ? offset : -EINVAL);
}

/* Relationship between i_mode and the DT_xxx types */
@@ -128,47 +143,44 @@ int dcache_readdir(struct file * filp, v
struct dentry *cursor = filp->private_data;
struct list_head *p, *q = &cursor->d_child;
ino_t ino;
- int i = filp->f_pos;

- switch (i) {
- case 0:
- ino = dentry->d_inode->i_ino;
- if (filldir(dirent, ".", 1, i, ino, DT_DIR) < 0)
- break;
- filp->f_pos++;
- i++;
- /* fallthrough */
- case 1:
- ino = parent_ino(dentry);
- if (filldir(dirent, "..", 2, i, ino, DT_DIR) < 0)
- break;
- filp->f_pos++;
- i++;
- /* fallthrough */
- default:
- spin_lock(&dcache_lock);
- if (filp->f_pos == 2) {
- list_del(q);
- list_add(q, &dentry->d_subdirs);
- }
- for (p=q->next; p != &dentry->d_subdirs; p=p->next) {
- struct dentry *next;
- next = list_entry(p, struct dentry, d_child);
- if (d_unhashed(next) || !next->d_inode)
- continue;
-
- spin_unlock(&dcache_lock);
- if (filldir(dirent, next->d_name.name, next->d_name.len, filp->f_pos, next->d_inode->i_ino, dt_type(next->d_inode)) < 0)
- return 0;
- spin_lock(&dcache_lock);
- /* next is still alive */
- list_del(q);
- list_add(q, p);
- p = q;
- filp->f_pos++;
- }
- spin_unlock(&dcache_lock);
+// if (IS_ERR(cursor->d_fsdata))
+// return PTR_ERR(cursor->d_fsdata);
+
+ if (filp->f_pos == 0) {
+ ino = dentry->d_inode->i_ino;
+ if (filldir(dirent, ".", 1, filp->f_pos, ino, DT_DIR) < 0)
+ goto out;
+ filp->f_pos = full_name_hash(".", 1);
}
+ if (filp->f_pos == full_name_hash(".", 1)) {
+ ino = parent_ino(dentry);
+ if (filldir(dirent, "..", 2, filp->f_pos, ino, DT_DIR) < 0)
+ goto out;
+ filp->f_pos = full_name_hash("..", 2);
+ }
+ spin_lock(&dcache_lock);
+ for (p=q->next; p != &dentry->d_subdirs; p=p->next) {
+ struct dentry *next;
+ next = list_entry(p, struct dentry, d_child);
+ if (d_unhashed(next) || !next->d_inode)
+ continue;
+ spin_unlock(&dcache_lock);
+ if (filldir(dirent, next->d_name.name,
+ next->d_name.len, filp->f_pos,
+ next->d_inode->i_ino,
+ dt_type(next->d_inode)) < 0)
+ goto out;
+ spin_lock(&dcache_lock);
+ /* next is still alive */
+ list_del(q);
+ list_add(q, p);
+ p = q;
+ filp->f_pos = full_name_hash(next->d_name.name,
+ next->d_name.len);
+ }
+ spin_unlock(&dcache_lock);
+out:
return 0;
}
\
 
 \ /
  Last update: 2005-11-04 12:45    [from the cache]
©2003-2014 Jasper Spaans. Advertise on this site