发件人 Cong WANG
收件人 linux-kernel@vger.kernel.org
日期 2007-3-11 下午10:15
主题 Style Question
邮送域 gmail.com
Hi, list!

I have a question about coding style in linux kernel. In
Documention/CodingStyle, it is said that “Linux style for comments is
the C89 “/ … /“ style. Don’t use C99-style “// …” comments.”
But I see a lot of ‘//‘ style comments in current kernel code.

Which is wrong? The documentions or the code, or neither? And why?

Another question is about NULL. AFAIK, in user space, using NULL is
better than directly using 0 in C. In kernel, I know it used its own
NULL, which may be defined as ((void*)0), but it’s still different
from raw zero. So can I say using NULL is better than 0 in kernel?

Any reply is welcome. Thanks and have a nice day!

Bernd Petrovitsch 第一个回答到：

On Sun, 2007-03-11 at 22:15 +0800, Cong WANG wrote:
[…]
> Another question is about NULL. AFAIK, in user space, using NULL is
> better than directly using 0 in C. In kernel, I know it used its own
> NULL, which may be defined as ((void*)0),

Userspace has the usually same definition.

> but it’s still different
> from raw zero.

It is different that “0” as such has the type “int”. But this int is
automatically promoted to a “0 pointer”.

> So can I say using NULL is better than 0 in kernel?

Yes, because it is immediately clear that a pointer is (or should be)
there (and not an int).
And the same holds for userspace since this is a pure C question.

Bernd

Jan Engelhardt 回复到：

On Mar 11 2007 22:15, Cong WANG wrote:
>
> I have a question about coding style in linux kernel. In
> Documention/CodingStyle, it is said that “Linux style for comments is
> the C89 “/ … /“ style. Don’t use C99-style “// …” comments.”
> But I see a lot of ‘//‘ style comments in current kernel code.
>
> Which is wrong? The documentions or the code, or neither? And why?

The code. And because it’s not always reviewed but silently pushed.

> Another question is about NULL. AFAIK, in user space, using NULL is
> better than directly using 0 in C. In kernel, I know it used its own
> NULL, which may be defined as ((void*)0), but it’s still different
> from raw zero.

In what way?

>So can I say using NULL is better than 0 in kernel?

On what basis? Do you even know what NULL is defined as in
(C, not C++) userspace? Think about it.

Jan

我看到后回复：

2007/3/12, Jan Engelhardt:
>
> On Mar 11 2007 22:15, Cong WANG wrote:
> >
> > I have a question about coding style in linux kernel. In
> > Documention/CodingStyle, it is said that “Linux style for comments is
> > the C89 “/ … /“ style. Don’t use C99-style “// …” comments.”
> > But I see a lot of ‘//‘ style comments in current kernel code.
> >
> > Which is wrong? The documentions or the code, or neither? And why?
>
> The code. And because it’s not always reviewed but silently pushed.
>
> > Another question is about NULL. AFAIK, in user space, using NULL is
> > better than directly using 0 in C. In kernel, I know it used its own
> > NULL, which may be defined as ((void*)0), but it’s still different
> > from raw zero.
>
> In what way?

The following code is picked from drivers/kvm/kvm_main.c:

static struct kvm_vcpu vcpu_load(struct kvm kvm, int vcpu_slot)
{
struct kvm_vcpu *vcpu = &kvm->vcpus[vcpu_slot];

mutex_lock(&vcpu->mutex);
if (unlikely(!vcpu->vmcs)) {
mutex_unlock(&vcpu->mutex);
return 0;
}
return kvm_arch_ops->vcpu_load(vcpu);
}

Obviously, it used 0 rather than NULL when returning a pointer to
indicate an error. Should we fix such issue?

>
> >So can I say using NULL is better than 0 in kernel?
>
> On what basis? Do you even know what NULL is defined as in
> (C, not C++) userspace? Think about it.
>

I think it’s more clear to indicate we are using a pointer rather than
an integer when we use NULL in kernel. But in userspace, using NULL is
for portbility of the program, although most (just most, NOT all) of
NULL’s defination is ((void*)0).

一些其它的回复如下：

Robert Hancock 写道：

Cong WANG wrote:
> Hi, list!
>
> I have a question about coding style in linux kernel. In
> Documention/CodingStyle, it is said that “Linux style for comments is
> the C89 “/ … /“ style. Don’t use C99-style “// …” comments.”
> But I see a lot of ‘//‘ style comments in current kernel code.
>
> Which is wrong? The documentions or the code, or neither? And why?

The code.. As with a lot of coding style issues, it’s likely just that
nobody saw it and bothered to complain when it went in.

> Another question is about NULL. AFAIK, in user space, using NULL is
> better than directly using 0 in C. In kernel, I know it used its own
> NULL, which may be defined as ((void*)0), but it’s still different
> from raw zero. So can I say using NULL is better than 0 in kernel?

It’s the preferred style, Sparse will complain about using 0 for a null
pointer for example..

Mac的Kyle Moffett 如是说：

On Mar 11, 2007, at 16:41:51, Daniel Hazelton wrote:
> On Sunday 11 March 2007 16:35:50 Jan Engelhardt wrote:
>> On Mar 11 2007 22:15, Cong WANG wrote:
>>> So can I say using NULL is better than 0 in kernel?
>>
>> On what basis? Do you even know what NULL is defined as in (C, not
>> C++) userspace? Think about it.
>
> IIRC, the glibc and GCC headers define NULL as (void*)0 :)

On the other hand when cplusplus is defined they define it to the
“null” builtin, which GCC uses to give type conversion errors for
“int foo = NULL” but not “char foo = NULL”. A “((void )0)”
definition gives C++ type errors for both due to the broken C++ void
pointer conversion problems.

Cheers,

Nicholas Miell这样说：

On Mon, 2007-03-12 at 06:40 +0100, Jan Engelhardt wrote:
> On Mar 12 2007 13:37, Cong WANG wrote:
> >
> > The following code is picked from drivers/kvm/kvm_main.c:
> >
> > static struct kvm_vcpu vcpu_load(struct kvm kvm, int vcpu_slot)
> > {
> > struct kvm_vcpu vcpu = &kvm->vcpus[vcpu_slot];
> >
> > mutex_lock(&vcpu->mutex);
> > if (unlikely(!vcpu->vmcs)) {
> > mutex_unlock(&vcpu->mutex);
> > return 0;
> > }
> > return kvm_arch_ops->vcpu_load(vcpu);
> > }
> >
> > Obviously, it used 0 rather than NULL when returning a pointer to
> > indicate an error. Should we fix such issue?
>
> Indeed. If it was for me, something like that should throw a compile error.
>
> >>[…]
> > I think it’s more clear to indicate we are using a pointer rather than
> > an integer when we use NULL in kernel. But in userspace, using NULL is
> > for portbility of the program, although most (just most, NOT all) of
> > NULL’s defination is ((void)0).

>
> NULL has the same bit pattern as the number zero. (I’m not saying the bit
> pattern is all zeroes. And I am not even sure if NULL ought to have the same
> pattern as zero.) So C++ could use (void *)0, if it would let itself :p

Not necessarily. You can use 0 at the source level, but the compiler has
to convert it to the actual NULL pointer bit pattern, whatever it may
be.

In C++, NULL is typically defined to 0 (with no void* cast) by most
compilers because 0 (and only 0) can be implicitly converted to to null
pointer of any ponter type without a cast.

GCC introduced the __null extension so that NULL still works correctly
in C++ when passed to a varargs function on 64-bit platforms.

(This just works in C because C makes NULL ((void)0) is thus is the
right size. In C++, the 0 ends up being an int instead of a pointer when
passed to a varargs function, and things tend to blow up when they read
the garbage high bits. Of course, nobody else does this, so you still
have to use (void)NULL to be portable.)

Randy.Dunlap 给以肯定回答：

On Mon, 12 Mar 2007, Jan Engelhardt wrote:

>
> On Mar 12 2007 13:37, Cong WANG wrote:
> >
> > The following code is picked from drivers/kvm/kvm_main.c:
> >
> > static struct kvm_vcpu vcpu_load(struct kvm kvm, int vcpu_slot)
> > {
> > struct kvm_vcpu *vcpu = &kvm->vcpus[vcpu_slot];
> >
> > mutex_lock(&vcpu->mutex);
> > if (unlikely(!vcpu->vmcs)) {
> > mutex_unlock(&vcpu->mutex);
> > return 0;
> > }
> > return kvm_arch_ops->vcpu_load(vcpu);
> > }
> >
> > Obviously, it used 0 rather than NULL when returning a pointer to
> > indicate an error. Should we fix such issue?
>
> Indeed. If it was for me, something like that should throw a compile error.

At least it does throw a sparse warning, and yes, it should
be fixed.

最后我决定提交补丁：

[PATCH]Replace 0 with NULL when returning a pointer

Use NULL to indicate we are returning a pointer rather than an integer
and to eliminate some sparse warnings.

Signed-off-by: Cong WANG <xiyou.wangcong@gmail.com>

---

—- drivers/kvm/kvm_main.c.orig 2007-03-11 21:41:23.000000000 +0800
+++ drivers/kvm/kvm_main.c 2007-03-12 14:26:17.000000000 +0800
@@ -205,7 +205,7 @@ static struct kvm_vcpu *vcpu_load(struct
mutex_lock(&vcpu->mutex);
if (unlikely(!vcpu->vmcs)) {
mutex_unlock(&vcpu->mutex);

• return 0;

• return NULL;
  }
  return kvm_arch_ops->vcpu_load(vcpu);
  }
  @@ -799,7 +799,7 @@ struct kvm_memory_slot *gfn_to_memslot(s
  && gfn < memslot->base_gfn + memslot->npages)
  return memslot;
  }

• return 0;

• return NULL;
  }
  EXPORT_SYMBOL_GPL(gfn_to_memslot);