On Tue, 20 Mar 2007, Thomas Gleixner wrote: > > + error = -ENFILE; > > + file = get_empty_filp(); > > + if (!file) > > + goto eexit_1; > > make this "return -ENFILE;" please
Done > > + inode = aino_getinode(); > > + if (IS_ERR(inode)) { > > + error = PTR_ERR(inode); > > + goto eexit_2; > > Can you please use a bit more descriptive labels ? > > e.g: > goto out_filp; Done > > +static int ainofs_delete_dentry(struct dentry *dentry) > > +{ > > + /* > > + * We faked vfs to believe the dentry was hashed when we created it. > > + * Now we restore the flag so that dput() will work correctly. > > + */ > > + dentry->d_flags |= DCACHE_UNHASHED; > > + return 1; > > +} > > Please put either "struct ainofs_dentry_operations ..." below the next > function or move ainofs_delete_dentry() above "struct > ainofs_dentry_operations ..." > > It's annoying to lookup the protoypes and implemenation back and forth. I prefer to have all data declarations at the beginning. but if you can manage to have that requirement in the Coding Style, I'll change it ;) > > +static struct inode *aino_getinode(void) > > +{ > > + return igrab(aino_inode); > > +} > > Please use "igrab(aino_inode);" directly in this one single place above. > That saves us a prototype and an useless static function with no value. Done > > +/* > > + * A single inode exist for all aino files. On the contrary of pipes, > > + * aino inodes has no per-instance data associated, so we can avoid > > + * the allocation of multiple of them. > > + */ > > +static struct inode *aino_mkinode(void) > > +{ > > + int error = -ENOMEM; > > + struct inode *inode = new_inode(aino_mnt->mnt_sb); > > + > > + if (!inode) > > + goto eexit_1; > > return ERR_PTR(-ENOMEM); Done > > + aino_mnt = kern_mount(&aino_fs_type); > > + if (IS_ERR(aino_mnt)) > > + goto epanic; > > + > > + aino_inode = aino_mkinode(); > > + if (IS_ERR(aino_inode)) > > + goto epanic; > > + > > + return 0; > > + > > +epanic: > > + panic("aino_init() failed\n"); > > Panic ? It's not life critical - is it ? > > A printk(KERN_ERR...) and a return -Exx would be sufficient. Done. - Davide - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/