Re: [RFC][PATCH 11/14] VFS whiteout handling

Jan Engelhardt
2007-05-16T08:07:54Z

On May 14 2007 15:13, Bharata B Rao wrote:
>
>+/*
>+ * Dummy default file-operations:
>+ * Never open a whiteout. This is always a bug.
>+ */
>+static int whiteout_no_open(struct inode *irrelevant, struct file *dontcare)
>+{
>+ printk("Attemp to open a whiteout!\n");
>+ WARN_ON(1);
>+ return -ENXIO;
>+}

Eww, really WARN_ON? ENXIO should be enough.
(FWIW, spello fix -> "Attempted to open a whiteout")

>+static int
>+vfs_unlink_whiteout(struct inode *dir, struct dentry **dp)

One line.

>+static int
>+__hash_one_len(const char *name, int len, struct qstr *this)
>+{
>+ unsigned long hash;
>+ unsigned int c;

I doubt it will make a difference - "unsigned char c";

>+
>+ hash = init_name_hash();
>+ while (len--) {
>+ c = *(const unsigned char *)name++;
>+ if (c == '/' || c == '\0')
>+ return -EINVAL;
>+ hash = partial_name_hash(c, hash);
>+ }
>+ this->hash = end_name_hash(hash);
>+ return 0;
>+}
>+
>+static int do_unlink_whiteouts(struct dentry *dentry)
>+{
>[...]
>+ if (!IS_DEADDIR(inode)) {
>+ res = file->f_op->readdir(file, (void *)file->f_path.dentry,
>+ unlink_whiteouts_filldir);

I think this cast can go.

>--- a/fs/readdir.c
>+++ b/fs/readdir.c
>@@ -148,6 +148,11 @@ static int filldir(void * __buf, const c
> unsigned long d_ino;
> int reclen = ALIGN(NAME_OFFSET(dirent) + namlen + 2, sizeof(long));
>
>+#ifdef CONFIG_UNION_MOUNT
>+ if (d_type == DT_WHT)
>+ return 0;
>+#endif /* CONFIG_UNION_MOUNT */
>+

DT_WHT is always available, is not it? In that case, would not it
be simpler to get rid of the #if/#endif.

>@@ -233,6 +238,11 @@ static int filldir64(void * __buf, const
> struct getdents_callback64 * buf = (struct getdents_callback64 *) __buf;
> int reclen = ALIGN(NAME_OFFSET(dirent) + namlen + 1, sizeof(u64));
>
>+#ifdef CONFIG_UNION_MOUNT
>+ if (d_type == DT_WHT)
>+ return 0;
>+#endif /* CONFIG_UNION_MOUNT */
>+
> buf->error = -EINVAL; /* only used if we fail.. */
> if (reclen > buf->count)
> return -EINVAL;

>--- a/fs/union.c
>+++ b/fs/union.c
>
> #define attach_mnt_union(mnt,nd) do { /* empty */ } while (0)
>@@ -54,6 +58,8 @@ extern int union_relookup_topmost(struct
> #define union_copy_file(dentry1,mnt1,dentry2,mnt2) ({ (0); })
> #define union_copyup(x,y) ({ (0); })
> #define union_relookup_topmost(x,y) ({ (0); })
>+#define union_dir_is_empty(x) ({ (1); })
>+#define present_in_lower(x, y) ({ (0); })

These could just be
#define something 0

Jan
--