tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: Seventh Edition(V7) filesystem support.



In article <20110623.214628.75173118806035116.uch%vnop.net@localhost>,
UCHIYAMA Yasushi  <uch%vnop.net@localhost> wrote:
>
>Done. I think. I decided to use progress function of fsck.
>How about this?
>ftp://ftp.netbsd.org/pub/NetBSD/misc/uch/
>v7fs-5.99.53-110623.patch
>v7fs-5.99.53-110623.tar.gz

Mostly done! Thanks, nitpicking:

more than one place:
        - sizeof(type) -> sizeof(*var) [many places]
          (changing the type in one place works)
        - setting error to valid errno values followed by warnx()
          are really errno values could set errno instead and call warn() to
          print the reason.
        - continuation lines in progress are not KNF.

fsck:
        - sprintf -> snprintf (we don't want to keep auditing unsafe functions)

mount:
        - mount_v7fs() should probably return EXIT_SUCCESS; instead of exit();

newfs:
        - while (condition); -> while (condition) continue;

kernel:
        - some 2nd line indents not KNF eg.
            if ((ret =
                    func(fs, ctx, blk, last ? *f

            blk = link(fs, link(fs, link(fs, addr[V7FS_NADDR_INDEX3],
                            map.index[0]),
                        map.index[1]),
                    map.index[2]);

            if (strncmp(p->name,
                        (const char *)dir->name, V7FS_NAME_MAX) == 0) {

            if ((error = v7fs_file_lookup_by_name(fs, parent_from, from,
                        &from_ino))) {

        - I would call the _16 and _32 mactos something more meaningful.

        - v7fs_mountroot() -> v7fs_mountroot(void) like everywhere else.
        
        - return (error); instead of return error;
        - function () instead of function() and sizeof () insteaof of sizeof()

        v7fs_datablock.c:    int (*) (struct v7fs_self *, void *, v7fs_daddr_t, 
size_t), void *);
        v7fs_datablock.c:    int (*) (struct v7fs_self *, void *, v7fs_daddr_t, 
size_t), void *);
        v7fs_datablock.c:    int (*func) (struct v7fs_self *, void *, 
v7fs_daddr_t, size_t), void *ctx)
        v7fs_datablock.c:    int (*func) (struct v7fs_self *, void *, 
v7fs_daddr_t, size_t),
        v7fs_datablock.c:    int (*func) (struct v7fs_self *, void *, 
v7fs_daddr_t, size_t), void *ctx)
        v7fs_file.c:    memset(&inode, 0, sizeof (struct v7fs_inode));
        v7fs_file.c:                        sizeof (struct v7fs_dirent) * 2))) {
        v7fs_file.c:                sizeof (struct v7fs_dirent) * 2 /*"." + 
".."*/) {
        v7fs_file.c:                sizeof (struct v7fs_dirent))))
        v7fs_file.c:    int n = sz / sizeof (struct v7fs_dirent) - 1;
        v7fs_file.c:    pos = lastsz - sizeof (struct v7fs_dirent);
        v7fs_file.c:    v7fs_datablock_contract(fs, parent_dir, sizeof (struct 
v7fs_dirent));
        v7fs_file_util.c:       n = sz / sizeof (struct v7fs_dirent);
        v7fs_file_util.c:       n = sz / sizeof (struct v7fs_dirent);
        v7fs_inode.c:   memset(&inode, 0, sizeof (struct v7fs_inode));
        v7fs_inode.c:       sizeof (struct v7fs_inode_diskimage);
        v7fs_io_kern.c: memset(p, 0, sizeof (struct v7fs_self));
        v7fs_io_user.c: memset(p, 0, sizeof (struct v7fs_self));
        v7fs_superblock.c:          sizeof (v7fs_daddr_t) * fb->nfreeblock);
        v7fs_superblock.c:      memcpy(to, from , sizeof (struct 
v7fs_superblock));
        v7fs_vnops.c:vtype_to_v7fs_mode (enum vtype type)
        v7fs_vnops.c:   attr.mode = va->va_mode | vtype_to_v7fs_mode 
(va->va_type);
        v7fs_vnops.c:   memset (&attr, 0, sizeof (struct v7fs_fileattr));
        v7fs_vnops.c:   v7fs_update (vp, 0, 0, UPDATE_WAIT);
        v7fs_vnops.c:           v7fs_inode_chmod (inode, vap->va_mode);
        v7fs_vnops.c:   v7fs_update (vp, 0, 0, 0);
        v7fs_vnops.c:   memset (&attr, 0, sizeof (struct v7fs_fileattr));
        v7fs_vnops.c:static int readdir_subr (struct v7fs_self *, void *, 
v7fs_daddr_t, size_t);
        v7fs_vnops.c:readdir_subr (struct v7fs_self *fs, void *ctx, 
v7fs_daddr_t blk, size_t sz)
        v7fs_vnops.c:               v7fs_inode_isdir (&inode) ? "DIR" : "FILE");
        v7fs_vnops.c:           v7fs_update (vp, 0, 0, UPDATE_WAIT);
        v7fs_vnops.c:   memset(&attr, 0, sizeof (struct v7fs_fileattr));
        v7fs_vnops.c:   scratch_free (fs, buf);

Fix those and commit! No need for a 3rd review.

christos



Home | Main Index | Thread Index | Old Index