On Mon, May 14, 2007 at 12:43:43PM +0200, Carsten Otte wrote:
> On 5/14/07, Bharata B Rao <bharata@xxxxxxxxxxxxxxxxxx> wrote:
> >+/* This is a copy from fs/readdir.c */
> >+struct getdents_callback {
> >+ struct linux_dirent __user *current_dir;
> >+ struct linux_dirent __user *previous;
> >+ int count;
> >+ int error;
> >+};
> This should go into a header file.
Yes ideally. As the comment above says, it is copied from fs/readdir.c and
we should be using the definition from there. But that needs touching
additional files and we wanted to avoid that for this initial RFC post.
>
> >+static int union_cache_find_entry(struct list_head *uc_list,
> >+ const char *name, int namelen)
> >+{
> >+ struct union_cache_entry *p;
> >+ int ret = 0;
> >+
> >+ list_for_each_entry(p, uc_list, list) {
> >+ if (p->name.len != namelen)
> >+ continue;
> >+ if (strncmp(p->name.name, name, namelen) == 0) {
> >+ ret = 1;
> >+ break;
> >+ }
> >+ }
> >+ return ret;
> >+}
> Why not use strlen instead of having both string and length as parameter?
>
All generic filldir routines in fs/readdir.c (filldir, fillonedir and
filldir64) don't depend on the dirent->d_name to be NULL terminated and
put a 0 themselves at the end. Hence we are also not depending on the
name string to be NULL terminated.
> >+static struct file * __dentry_open_read(struct dentry *dentry,
> >+ struct vfsmount *mnt, int flags)
> >+{
> >+ struct file *f;
> >+ struct inode *inode;
> >+ int error;
> >+
> >+ error = -ENFILE;
> >+ f = get_empty_filp();
> >+ if (!f)
> >+ goto out;
> This is the only case where error is not explicitly set to a different
> value before hitting out/cleanup => consider setting conditionally.
Sure can be done. Again this routine is copied from dentry_open() and
hence it is like that atm.
Thanks for your review.
Regards,
Bharata.
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
