From: "Daniel Aragonés" <danarag@gmail.com>
To: Pekka Enberg <penberg@cs.helsinki.fi>, linux-kernel@vger.kernel.org
Subject: Re: [PATCH/RFC] minix filesystem: Corrected patch
Date: Sun, 22 Jan 2006 21:22:04 +0100 [thread overview]
Message-ID: <43D3E96C.2050703@gmail.com> (raw)
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.
next reply other threads:[~2006-01-22 20:26 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2006-01-22 20:22 Daniel Aragonés [this message]
2006-01-22 20:37 ` [PATCH/RFC] minix filesystem: Corrected patch Pekka Enberg
-- strict thread matches above, loose matches on Subject: below --
2006-01-22 18:34 Daniel Aragonés
2006-01-22 19:17 ` Pekka Enberg
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=43D3E96C.2050703@gmail.com \
--to=danarag@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=penberg@cs.helsinki.fi \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).