From: Timothy Shimmin <tes@sgi.com>
To: Barry Naujok <bnaujok@sgi.com>
Cc: Chandan Talukdar <chandan@agami.com>,
"xfs@oss.sgi.com" <xfs@oss.sgi.com>
Subject: Re: [REVIEW] Refactor xfs_repair's process_dinode_int
Date: Wed, 16 Jan 2008 14:10:58 +1100 [thread overview]
Message-ID: <478D75C2.5010004@sgi.com> (raw)
In-Reply-To: <op.t4zzbvbs3jf8g2@pc-bnaujok.melbourne.sgi.com>
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@agami.com>
> 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
next prev parent reply other threads:[~2008-01-16 3:10 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <4782B72D.8070208@agami.com>
[not found] ` <47833C0F.6070206@agami.com>
[not found] ` <op.t4m0r1an3jf8g2@pc-bnaujok.melbourne.sgi.com>
[not found] ` <478D1899.9080201@agami.com>
2008-01-16 0:51 ` [REVIEW] Refactor xfs_repair's process_dinode_int Barry Naujok
2008-01-16 2:33 ` Barry Naujok
2008-01-16 3:10 ` Timothy Shimmin [this message]
2008-01-16 4:59 ` David Chinner
2008-01-16 6:32 ` Christoph Hellwig
2007-11-15 6:40 Barry Naujok
2007-11-21 15:05 ` Christoph Hellwig
2008-01-16 1:03 ` Michael Nishimoto
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=478D75C2.5010004@sgi.com \
--to=tes@sgi.com \
--cc=bnaujok@sgi.com \
--cc=chandan@agami.com \
--cc=xfs@oss.sgi.com \
/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