Messages in this thread | | | Date | Sun, 22 Jan 2006 21:22:04 +0100 | From | Daniel Aragonés <> | Subject | Re: [PATCH/RFC] minix filesystem: Corrected patch |
| |
Hi Pekka!
On 1/22/06, you wrote:
>+ offset = p - kaddr; >> + over = filldir(dirent, de3->name, l, >> + (n<<PAGE_CACHE_SHIFT) | offset, >> + de3->inode, DT_UNKNOWN); >Hmm, strange formatting. Wouldn't it be better if you introduced a >name pointer and moved those filldir bits outside of the if-else >block? Less code duplication that way. >+ if (namecompare(namelen,sbi->s_namelen,name,de3->name)) >> + goto found; >> + } >Same here. >+ goto out_unlock; >> + de = minix_next_entry(de, sbi); >> + de3 = minix_next_entry(de3, sbi); >Why do you do both here?
You are right, but I thought that duplication was the appropiate to be the most conservative with the preexistent code and also providing for the needed duplication of the strucutre minix_dir_entry. The secondary structure (minix3_dir_entry) has to follow all the endeavours of its parent one, so both are here.
>+ sbi->s_log_zone_size = *(__u16 *)(bh->b_data + 12); >> + sbi->s_max_size = *(__u32 *)(bh->b_data + 16); >> + sbi->s_nzones = *(__u32 *)(bh->b_data + 20); >You probably want to introduce a struct minix3_super_block for this. >It's much more readable that way.
Yes, but if I do, is closer to a rewrite of the preexistent code. And I think that it not deserves it. Minix is not so important (sorry if some one is listening).
>+ goto out_bad_hblock; >> + } >You're now setting the block size twice for the V3 case.
You are right.
>+#define MINIX2_INODES_PER_BLOCK(b) ((b)/(sizeof (struct minix2_inode))) >Maybe this should be called minix_inodes_per_block instead and be a >static inline function?
Just to follow the style found.
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |