On Tue, Mar 26, 2013 at 03:19:29PM +0000, James Hogan wrote:
> +struct da_stat {
> + short st_dev;
> + unsigned short st_ino;
> + unsigned st_mode;
> + unsigned short st_nlink;
> + unsigned short st_uid;
> + unsigned short st_gid;
> + short st_rdev;
> + int st_size;
> + int st_atime;
> + int st_spare1;
> + int st_mtime;
> + int st_spare2;
> + int st_ctime;
> + int st_spare3;
> + int st_blksize;
> + int st_blocks;
> + int st_spare4[2];
> +};
Please, use explicitly-sized types.
> +struct da_finddata {
> + unsigned long size;
> + unsigned long attrib;
> + char name[260];
> +};
... especially for this - unlike int and short, long really is arch-dependent.
And yes, I realize that this thing is arch-specific, but...
> +struct dafs_inode_info {
> + int fd;
> + int mode;
umm... int or fmode_t?
> + struct inode vfs_inode;
> +};
> +#define DAFS_SUPER_MAGIC 0xdadadaf5
-> linux/magic.h
> +static char *dentry_name(struct dentry *dentry)
> +{
> + char *name = __getname();
> + if (!name)
> + return NULL;
> +
> + return __dentry_name(dentry, name); /* will unlock */
will unlock what?
> + hi = kzalloc(sizeof(*hi), GFP_KERNEL);
> + if (hi == NULL)
> + return NULL;
> +
> + hi->fd = -1;
> + inode_init_once(&hi->vfs_inode);
> + return &hi->vfs_inode;
Umm... kzalloc() looks odd, seeing that you proceed to initialize two
fields out of 3 (including the really big one) explicitly...
> +static int open_dir(char *path, struct da_finddata *finddata, int *fserrno)
> +{
> + int len = strlen(path);
> + char buf[len + 3];
Bad idea, considering that strlen(path) is user-controlled and can
theoretically go up to 4Kb...
> +static int dafs_readdir(struct file *file, void *ent, filldir_t filldir)
> +{
> + struct inode *inode = file->f_path.dentry->d_inode;
minor nit, but... that's file_inode(file)
> + name = dentry_name(file->f_path.dentry);
> + if (name == NULL)
> + return -ENOMEM;
> + handle = open_dir(name, &finddata, &fserrno);
... and what if the sucker's parent gets renamed between dentry_name() and
open_dir()? The same goes for the rest of dentry_name() users, actually.
> +static int dafs_rename(struct inode *from_ino, struct dentry *from,
> + struct inode *to_ino, struct dentry *to)
> +{
> + char *from_name, *to_name;
> + int err;
> +
> + from_name = dentry_name(from);
> + if (from_name == NULL)
> + return -ENOMEM;
> + to_name = dentry_name(to);
> + if (to_name == NULL) {
> + __putname(from_name);
> + return -ENOMEM;
> + }
> + err = -EINVAL;
> + __putname(from_name);
> + __putname(to_name);
> + return err;
> +}
Aha. No rename(2), IOW. Is one planned?
--
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
