[Ocfs2-devel] Re: [Ocfs2-commits] manish commits r2175 - in trunk/fs/ocfs2: . cluster dlm

[Ocfs2-devel] Re: [Ocfs2-commits] manish commits r2175 - in trunk/fs/ocfs2: . cluster dlm

[Christoph Hellwig]

On Mon, Apr 25, 2005 at 10:23:12PM -0500, svn-commits@oss.oracle.com wrote:
> Author: manish
> Signed-off-by: mfasheh
> Date: 2005-04-25 22:23:11 -0500 (Mon, 25 Apr 2005)
> New Revision: 2175
> 
> Modified:
>    trunk/fs/ocfs2/alloc.c
>    trunk/fs/ocfs2/aops.c
>    trunk/fs/ocfs2/buffer_head_io.c
>    trunk/fs/ocfs2/cluster/masklog.h
>    trunk/fs/ocfs2/dcache.c
>    trunk/fs/ocfs2/dir.c
>    trunk/fs/ocfs2/dlm/dlmast.c
>    trunk/fs/ocfs2/dlm/dlmconvert.c
>    trunk/fs/ocfs2/dlm/dlmdebug.c
>    trunk/fs/ocfs2/dlm/dlmrecovery.c
>    trunk/fs/ocfs2/dlm/dlmunlock.c
>    trunk/fs/ocfs2/dlmglue.c
>    trunk/fs/ocfs2/file.c
>    trunk/fs/ocfs2/inode.c
>    trunk/fs/ocfs2/journal.c
>    trunk/fs/ocfs2/localalloc.c
>    trunk/fs/ocfs2/mmap.c
>    trunk/fs/ocfs2/namei.c
>    trunk/fs/ocfs2/ocfs.h
>    trunk/fs/ocfs2/proc.c
>    trunk/fs/ocfs2/suballoc.c
>    trunk/fs/ocfs2/super.c
>    trunk/fs/ocfs2/vote.c
> Log:
> Define MLFu64 and friends, for portable format strings for sized 64-bit types.
> This gets rid of the tons on warnings on ia64 and ppc64.

Standard kernel practice is to cast a u64 to unsigned long (and a s64 to
long), and not using such obsfucation.  I'd strongly suggest to follow
that lead in ocfs.

[Ocfs2-devel] Re: [Ocfs2-commits] manish commits r2175 - in trunk/fs/ocfs2: . cluster dlm

[Manish Singh]

On Tue, Apr 26, 2005 at 09:18:26AM +0200, Christoph Hellwig wrote:
> On Mon, Apr 25, 2005 at 10:23:12PM -0500, svn-commits@oss.oracle.com wrote:
> > Log:
> > Define MLFu64 and friends, for portable format strings for sized 64-bit types.
> > This gets rid of the tons on warnings on ia64 and ppc64.
> 
> Standard kernel practice is to cast a u64 to unsigned long (and a s64 to
> long), and not using such obsfucation.  I'd strongly suggest to follow
> that lead in ocfs.

I assume you meant to say "unsigned long long" and "long long".

The issue here is format string readability vs. printk argument
readability. Both magic format defines and verbose (unsigned long long)
casts make the code harder to read. Why is format string readability
preferred?

The nice thing about Documentation/CodingStyle is that it provides
cogent rationales for each of coding style precepts. I'm perfectly
willing to entertain the idea that magic format defines are horrible,
but having a good "why" helps the idea take root.


-Manish

[Ocfs2-devel] Re: [Ocfs2-commits] manish commits r2175 - in trunk/fs/ocfs2: . cluster dlm

[Christoph Hellwig]

On Tue, Apr 26, 2005 at 01:15:38AM -0700, Manish Singh wrote:
> > Standard kernel practice is to cast a u64 to unsigned long (and a s64 to
> > long), and not using such obsfucation.  I'd strongly suggest to follow
> > that lead in ocfs.
> 
> I assume you meant to say "unsigned long long" and "long long".

Yes.

> The issue here is format string readability vs. printk argument
> readability. Both magic format defines and verbose (unsigned long long)
> casts make the code harder to read. Why is format string readability
> preferred?

I don't know ;-)  But if magic format strings we preferred we'd have
them in a common place.

[Ocfs2-devel] Re: [Ocfs2-commits] manish commits r2175 - in trunk/fs/ocfs2: . cluster dlm

[Manish Singh]

On Tue, Apr 26, 2005 at 10:20:33AM +0200, Christoph Hellwig wrote:
> On Tue, Apr 26, 2005 at 01:15:38AM -0700, Manish Singh wrote:
> > The issue here is format string readability vs. printk argument
> > readability. Both magic format defines and verbose (unsigned long long)
> > casts make the code harder to read. Why is format string readability
> > preferred?
> 
> I don't know ;-)  But if magic format strings we preferred we'd have
> them in a common place.

Perhaps they should be in some common place then? The kernel could
provide the C99 macros for dealing with explicitly sized types.

It sounds like the reason it doesn't exist already is simply because
nobody has done it yet. Printing 64-bit types in non-arch specific code
is rare in the current kernel tree, which is probably why nobody has
cared yet. I don't really see any evidence of a specific precedent set
against doing so.

-Manish

[Ocfs2-devel] Re: [Ocfs2-commits] manish commits r2175 - in trunk/fs/ocfs2: . cluster dlm

[Christoph Hellwig]

On Tue, Apr 26, 2005 at 01:47:53AM -0700, Manish Singh wrote:
> Perhaps they should be in some common place then? The kernel could
> provide the C99 macros for dealing with explicitly sized types.

I don't think we wanted this, but feel free to bring it up on lkml
again.

> It sounds like the reason it doesn't exist already is simply because
> nobody has done it yet. Printing 64-bit types in non-arch specific code
> is rare in the current kernel tree,

There are lots of places printinkg 64bit values in filesystem and block
layer code.