xfs
[Top] [All Lists]

Re: [REVIEW] Refactor xfs_repair's process_dinode_int

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@pc-bnaujok.melbourne.sgi.com>
References: <4782B72D.8070208@agami.com> <47833C0F.6070206@agami.com> <op.t4m0r1an3jf8g2@pc-bnaujok.melbourne.sgi.com> <478D1899.9080201@agami.com> <op.t4zzbvbs3jf8g2@pc-bnaujok.melbourne.sgi.com>
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>