| To: | Barry Naujok <bnaujok@xxxxxxx> |
|---|---|
| Subject: | Re: [REVIEW] Refactor xfs_repair's process_dinode_int |
| From: | Timothy Shimmin <tes@xxxxxxx> |
| Date: | Wed, 16 Jan 2008 14:10:58 +1100 |
| Cc: | Chandan Talukdar <chandan@xxxxxxxxx>, "xfs@xxxxxxxxxxx" <xfs@xxxxxxxxxxx> |
| In-reply-to: | <op.t4zzbvbs3jf8g2@xxxxxxxxxxxxxxxxxxxxxxxxxxxx> |
| References: | <4782B72D.8070208@xxxxxxxxx> <47833C0F.6070206@xxxxxxxxx> <op.t4m0r1an3jf8g2@xxxxxxxxxxxxxxxxxxxxxxxxxxxx> <478D1899.9080201@xxxxxxxxx> <op.t4zzbvbs3jf8g2@xxxxxxxxxxxxxxxxxxxxxxxxxxxx> |
| Sender: | xfs-bounce@xxxxxxxxxxx |
| User-agent: | Thunderbird 2.0.0.9 (Macintosh/20071031) |
Not that it is a big deal....but my 2 cents... Barry Naujok wrote: On Wed, 16 Jan 2008 07:33:29 +1100, Chandan Talukdar <chandan@xxxxxxxxx> wrote:Hi Barry,- In process_misc_ino_types(), dino->di_core.di_size is being accessed without being converted to machine format. The check is being performed against 0; so, it should be fine. But for better code readability, I guess it should be accessed through be64_to_cpu().Yeah... sort of in two-minds about this one. Well, traditionally we would not be endian converting it. We don't endian convert things which are compared to zero or are only 1 byte. There are a bunch of examples in the kernel code (many Christoph has done) and we should be consistent IMHO. (There is, of course, no point from a code point of view - I guess you might consider that you are letting people know that we need to endian convert this value in general and that if we change the code in the future it might be needed... but just say no.:) --Tim |
| <Prev in Thread] | Current Thread | [Next in Thread> |
|---|---|---|
| ||
| Previous by Date: | Re: [REVIEW] Refactor xfs_repair's process_dinode_int, Barry Naujok |
|---|---|
| Next by Date: | Re: [REVIEW] Refactor xfs_repair's process_dinode_int, David Chinner |
| Previous by Thread: | Re: [REVIEW] Refactor xfs_repair's process_dinode_int, Barry Naujok |
| Next by Thread: | Re: [REVIEW] Refactor xfs_repair's process_dinode_int, David Chinner |
| Indexes: | [Date] [Thread] [Top] [All Lists] |