From: Coly Li <coyli@suse.de>
To: Andreas Dilger <adilger@clusterfs.com>
Cc: linux-ext4@vger.kernel.org
Subject: Re: [PATCH] e2fsprogs: remove fragment support which will never be implemented [modified format]
Date: Fri, 10 Aug 2007 21:19:44 +0800 [thread overview]
Message-ID: <46BC65F0.5070107@suse.de> (raw)
In-Reply-To: <20070810092829.GN6689@schatzie.adilger.int>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Andreas Dilger wrote:
> On Aug 10, 2007 11:37 +0800, Coly Li wrote:
>> - --- a/e2fsck/problem.h
>> +++ b/e2fsck/problem.h
>> @@ -558,8 +558,8 @@ struct problem_context {
>> +/* Duplicate directory entry found */
>> +#define PR_2_REPORT_DUP_DIRENT 0x02000D
>>
>> - -/* i_frag should be zero */
>> - -#define PR_2_FRAG_ZERO 0x020010
>> +/* Non-unique filename found */
>> +#define PR_2_NON_UNIQUE_FILE 0x020010
>>
>> - -/* i_fsize should be zero */
>> - -#define PR_2_FSIZE_ZERO 0x020011
>> +/* i_blocks_hi should be zero */
>> +#define PR_2_BLOCKS_HI_ZERO 0x020011
>
> Please don't do this. This makes other patches fail to apply, and I don't
> think we need to have sequential error numbers?
Yes, I should only remove the obsoleted macro. BTW, why not use enum type to declare the error number ?
>> struct {
>> - - __u8 m_i_frag; /* Fragment number */
>> - - __u8 m_i_fsize; /* Fragment size */
>> - - __u16 m_pad1;
>> - - __u32 m_i_reserved2[2];
>> + __u32 m_i_reserved2[3];
>
> It is a bad idea to declare on-disk fields "reserved[{num}]", because
> if some code is ever using e.g. "reserved[0]" for something and then
> one of the fields is "unreserved" then the other code will silently
> continue to work, but it will be using some other field on disk...
>
> That said, while we are removing junk you could also remove the "masix"
> parts of the code, because I don't think they have been used for 10 years.
Sure, it seems that it will be better to remove all this structure. Isn't it ?
>
> Cheers, Andreas
> --
> Andreas Dilger
> Principal Software Engineer
> Cluster File Systems, Inc.
>
Andreas, thanks for your comments.
- --
Coly Li
SuSE PRC Labs
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2 (GNU/Linux)
Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org
iD8DBQFGvGXwuTp8cyZ5lTERAqu9AJwIkWMO/2OjxD2teOh76d6wQ3M6nACeN1rU
sy8hm6ugCJTB4z+D/0foTsU=
=/ZXl
-----END PGP SIGNATURE-----
prev parent reply other threads:[~2007-08-10 13:19 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-08-10 3:37 [PATCH] e2fsprogs: remove fragment support which will never be implemented [modified format] Coly Li
2007-08-10 9:28 ` Andreas Dilger
2007-08-10 13:19 ` Coly Li [this message]
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=46BC65F0.5070107@suse.de \
--to=coyli@suse.de \
--cc=adilger@clusterfs.com \
--cc=linux-ext4@vger.kernel.org \
/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).