[PATCH] Use NULL instead of integer 0 in security/selinux/

[PATCH] Use NULL instead of integer 0 in security/selinux/

[Chris Wright]

Fixup another round of sparse warnings of the type:
	warning: Using plain integer as NULL pointer
Acked by Stephen.  Rediffed against bk-current.

From: Mika Kukkonen <mika@osdl.org>
Signed-off-by: Chris Wright <chrisw@osdl.org>

===== security/selinux/avc.c 1.13 vs edited =====
--- 1.13/security/selinux/avc.c	2004-07-05 03:32:34 -07:00
+++ edited/security/selinux/avc.c	2004-07-07 12:15:48 -07:00
@@ -106,7 +106,7 @@
  */
 void avc_dump_av(struct audit_buffer *ab, u16 tclass, u32 av)
 {
-	char **common_pts = 0;
+	char **common_pts = NULL;
 	u32 common_base = 0;
 	int i, i2, perm;
 
@@ -734,7 +734,7 @@
 		}
 	} else {
 		/* apply to one node */
-		node = avc_search_node(ssid, tsid, tclass, 0);
+		node = avc_search_node(ssid, tsid, tclass, NULL);
 		if (node) {
 			avc_update_node(event,node,perms);
 		}
@@ -808,7 +808,7 @@
                  u32 perms, u32 seqno)
 {
 	return avc_control(AVC_CALLBACK_GRANT,
-			   ssid, tsid, tclass, perms, seqno, 0);
+			   ssid, tsid, tclass, perms, seqno, NULL);
 }
 
 /**
@@ -846,7 +846,7 @@
                   u32 perms, u32 seqno)
 {
 	return avc_control(AVC_CALLBACK_REVOKE,
-			   ssid, tsid, tclass, perms, seqno, 0);
+			   ssid, tsid, tclass, perms, seqno, NULL);
 }
 
 /**
@@ -878,7 +878,7 @@
 			avc_node_freelist = tmp;
 			avc_cache.active_nodes--;
 		}
-		avc_cache.slots[i] = 0;
+		avc_cache.slots[i] = NULL;
 	}
 	avc_cache.lru_hint = 0;
 
@@ -890,7 +890,7 @@
 	for (c = avc_callbacks; c; c = c->next) {
 		if (c->events & AVC_CALLBACK_RESET) {
 			rc = c->callback(AVC_CALLBACK_RESET,
-					 0, 0, 0, 0, 0);
+					 0, 0, 0, 0, NULL);
 			if (rc)
 				goto out;
 		}
@@ -918,10 +918,10 @@
 {
 	if (enable)
 		return avc_control(AVC_CALLBACK_AUDITALLOW_ENABLE,
-				   ssid, tsid, tclass, perms, seqno, 0);
+				   ssid, tsid, tclass, perms, seqno, NULL);
 	else
 		return avc_control(AVC_CALLBACK_AUDITALLOW_DISABLE,
-				   ssid, tsid, tclass, perms, seqno, 0);
+				   ssid, tsid, tclass, perms, seqno, NULL);
 }
 
 /**
@@ -938,10 +938,10 @@
 {
 	if (enable)
 		return avc_control(AVC_CALLBACK_AUDITDENY_ENABLE,
-				   ssid, tsid, tclass, perms, seqno, 0);
+				   ssid, tsid, tclass, perms, seqno, NULL);
 	else
 		return avc_control(AVC_CALLBACK_AUDITDENY_DISABLE,
-				   ssid, tsid, tclass, perms, seqno, 0);
+				   ssid, tsid, tclass, perms, seqno, NULL);
 }
 
 /**
@@ -993,7 +993,7 @@
 			ae->used = 1;
 		} else {
 			avc_cache_stats_incr(AVC_ENTRY_DISCARDS);
-			ae = 0;
+			ae = NULL;
 		}
 	}
 
===== security/selinux/ss/conditional.c 1.1 vs edited =====
--- 1.1/security/selinux/ss/conditional.c	2004-03-16 02:29:22 -08:00
+++ edited/security/selinux/ss/conditional.c	2004-07-07 12:15:48 -07:00
@@ -217,7 +217,7 @@
 
 int cond_read_bool(struct policydb *p, struct hashtab *h, void *fp)
 {
-	char *key = 0;
+	char *key = NULL;
 	struct cond_bool_datum *booldatum;
 	__u32 *buf, len;
 
@@ -251,7 +251,7 @@
 
 	return 0;
 err:
-	cond_destroy_bool(key, booldatum, 0);
+	cond_destroy_bool(key, booldatum, NULL);
 	return -1;
 }
 
===== security/selinux/ss/ebitmap.c 1.3 vs edited =====
--- 1.3/security/selinux/ss/ebitmap.c	2003-12-30 00:40:54 -08:00
+++ edited/security/selinux/ss/ebitmap.c	2004-07-07 12:15:48 -07:00
@@ -17,7 +17,7 @@
 
 	n1 = e1->node;
 	n2 = e2->node;
-	prev = 0;
+	prev = NULL;
 	while (n1 || n2) {
 		new = kmalloc(sizeof(*new), GFP_ATOMIC);
 		if (!new) {
@@ -40,7 +40,7 @@
 			n2 = n2->next;
 		}
 
-		new->next = 0;
+		new->next = NULL;
 		if (prev)
 			prev->next = new;
 		else
@@ -80,7 +80,7 @@
 
 	ebitmap_init(dst);
 	n = src->node;
-	prev = 0;
+	prev = NULL;
 	while (n) {
 		new = kmalloc(sizeof(*new), GFP_ATOMIC);
 		if (!new) {
@@ -90,7 +90,7 @@
 		memset(new, 0, sizeof(*new));
 		new->startbit = n->startbit;
 		new->map = n->map;
-		new->next = 0;
+		new->next = NULL;
 		if (prev)
 			prev->next = new;
 		else
@@ -155,7 +155,7 @@
 {
 	struct ebitmap_node *n, *prev, *new;
 
-	prev = 0;
+	prev = NULL;
 	n = e->node;
 	while (n && n->startbit <= bit) {
 		if ((n->startbit + MAPSIZE) > bit) {
@@ -231,7 +231,7 @@
 	}
 
 	e->highbit = 0;
-	e->node = 0;
+	e->node = NULL;
 	return;
 }
 
===== security/selinux/ss/mls.c 1.4 vs edited =====
--- 1.4/security/selinux/ss/mls.c	2004-06-03 01:46:38 -07:00
+++ edited/security/selinux/ss/mls.c	2004-07-07 12:15:48 -07:00
@@ -654,7 +654,7 @@
 
 int sens_read(struct policydb *p, struct hashtab *h, void *fp)
 {
-	char *key = 0;
+	char *key = NULL;
 	struct level_datum *levdatum;
 	int rc;
 	u32 *buf, len;
@@ -707,7 +707,7 @@
 
 int cat_read(struct policydb *p, struct hashtab *h, void *fp)
 {
-	char *key = 0;
+	char *key = NULL;
 	struct cat_datum *catdatum;
 	int rc;
 	u32 *buf, len;
===== security/selinux/ss/policydb.c 1.12 vs edited =====
--- 1.12/security/selinux/ss/policydb.c	2004-06-18 11:43:31 -07:00
+++ edited/security/selinux/ss/policydb.c	2004-07-07 12:15:48 -07:00
@@ -99,7 +99,7 @@
  */
 int roles_init(struct policydb *p)
 {
-	char *key = 0;
+	char *key = NULL;
 	int rc;
 	struct role_datum *role;
 
@@ -402,7 +402,7 @@
 
 	kfree(key);
 	comdatum = datum;
-	hashtab_map(comdatum->permissions.table, perm_destroy, 0);
+	hashtab_map(comdatum->permissions.table, perm_destroy, NULL);
 	hashtab_destroy(comdatum->permissions.table);
 	kfree(datum);
 	return 0;
@@ -416,7 +416,7 @@
 
 	kfree(key);
 	cladatum = datum;
-	hashtab_map(cladatum->permissions.table, perm_destroy, 0);
+	hashtab_map(cladatum->permissions.table, perm_destroy, NULL);
 	hashtab_destroy(cladatum->permissions.table);
 	constraint = cladatum->constraints;
 	while (constraint) {
@@ -498,7 +498,7 @@
 	int i;
 
 	for (i = 0; i < SYM_NUM; i++) {
-		hashtab_map(p->symtab[i].table, destroy_f[i], 0);
+		hashtab_map(p->symtab[i].table, destroy_f[i], NULL);
 		hashtab_destroy(p->symtab[i].table);
 	}
 
@@ -669,7 +669,7 @@
 
 static int perm_read(struct policydb *p, struct hashtab *h, void *fp)
 {
-	char *key = 0;
+	char *key = NULL;
 	struct perm_datum *perdatum;
 	int rc;
 	u32 *buf, len;
@@ -718,7 +718,7 @@
 
 static int common_read(struct policydb *p, struct hashtab *h, void *fp)
 {
-	char *key = 0;
+	char *key = NULL;
 	struct common_datum *comdatum;
 	u32 *buf, len, nel;
 	int i, rc;
@@ -776,7 +776,7 @@
 
 static int class_read(struct policydb *p, struct hashtab *h, void *fp)
 {
-	char *key = 0;
+	char *key = NULL;
 	struct class_datum *cladatum;
 	struct constraint_node *c, *lc;
 	struct constraint_expr *e, *le;
@@ -943,7 +943,7 @@
 
 static int role_read(struct policydb *p, struct hashtab *h, void *fp)
 {
-	char *key = 0;
+	char *key = NULL;
 	struct role_datum *role;
 	int rc;
 	u32 *buf, len;
@@ -1008,7 +1008,7 @@
 
 static int type_read(struct policydb *p, struct hashtab *h, void *fp)
 {
-	char *key = 0;
+	char *key = NULL;
 	struct type_datum *typdatum;
 	int rc;
 	u32 *buf, len;
@@ -1055,7 +1055,7 @@
 
 static int user_read(struct policydb *p, struct hashtab *h, void *fp)
 {
-	char *key = 0;
+	char *key = NULL;
 	struct user_datum *usrdatum;
 	int rc;
 	u32 *buf, len;
===== security/selinux/ss/services.c 1.16 vs edited =====
--- 1.16/security/selinux/ss/services.c	2004-06-18 11:43:31 -07:00
+++ edited/security/selinux/ss/services.c	2004-07-07 12:15:48 -07:00
@@ -308,7 +308,7 @@
 			u32 requested,
 			struct av_decision *avd)
 {
-	struct context *scontext = 0, *tcontext = 0;
+	struct context *scontext = NULL, *tcontext = NULL;
 	int rc = 0;
 
 	if (!ss_initialized) {
@@ -355,7 +355,7 @@
 {
 	char *scontextp;
 
-	*scontext = 0;
+	*scontext = NULL;
 	*scontext_len = 0;
 
 	/* Compute the size of the context. */
@@ -600,8 +600,8 @@
 				u32 specified,
 				u32 *out_sid)
 {
-	struct context *scontext = 0, *tcontext = 0, newcontext;
-	struct role_trans *roletr = 0;
+	struct context *scontext = NULL, *tcontext = NULL, newcontext;
+	struct role_trans *roletr = NULL;
 	struct avtab_key avkey;
 	struct avtab_datum *avdatum;
 	struct avtab_node *node;

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Herbert Xu]

Chris Wright <chrisw@osdl.org> wrote:
> Fixup another round of sparse warnings of the type:
>        warning: Using plain integer as NULL pointer

What's wrong with using 0 as the NULL pointer? In contexts where
a plain 0 is unsafe, NULL is usually unsafe as well.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[David S. Miller]

On Thu, 08 Jul 2004 13:12:41 +1000
Herbert Xu <herbert@gondor.apana.org.au> wrote:

> What's wrong with using 0 as the NULL pointer? In contexts where
> a plain 0 is unsafe, NULL is usually unsafe as well.

It's a general sparse cleanup people are doing across the
entire tree.  It's the "proper" way to do pointer comparisons
post-K&R.

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Miles Bader]

"David S. Miller" <davem@redhat.com> writes:
>> What's wrong with using 0 as the NULL pointer? In contexts where
>> a plain 0 is unsafe, NULL is usually unsafe as well.
>
> It's a general sparse cleanup people are doing across the entire tree.
> It's the "proper" way to do pointer comparisons post-K&R.

But 0 in such a context isn't an integer, it's a pointer...

If sparse really warns about such things, the warning seems wrong.

-Miles
-- 
((lambda (x) (list x x)) (lambda (x) (list x x)))

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Linus Torvalds]

On Thu, 8 Jul 2004, Miles Bader wrote:

> "David S. Miller" <davem@redhat.com> writes:
> >> What's wrong with using 0 as the NULL pointer? In contexts where
> >> a plain 0 is unsafe, NULL is usually unsafe as well.
> >
> > It's a general sparse cleanup people are doing across the entire tree.
> > It's the "proper" way to do pointer comparisons post-K&R.
> 
> But 0 in such a context isn't an integer, it's a pointer...

No it's not.

I'm sorry that you are such a K&R-C bigot that you don't like type 
checking. But the kernel DOES like type checking, and the kernel is not 
K&R C. The kernel uses strict ANSI, and in fact, is _more_ strict than 
ANSI C is in many many ways.

One of the "strict typechecking" rules is that you don't mix integers and 
pointers by mistake. The fact that C allows dual usage of the integer "0"
is an anachronism that should have been fixed long ago.

Final word: K&R C without prototypes etc is still "legal C". That doesn't
make it legal kernel code.

		Linus

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Miles Bader]

Linus Torvalds <torvalds@osdl.org> writes:
>> >> What's wrong with using 0 as the NULL pointer? In contexts where
>> >> a plain 0 is unsafe, NULL is usually unsafe as well.
>> >
>> > It's a general sparse cleanup people are doing across the entire tree.
>> > It's the "proper" way to do pointer comparisons post-K&R.
>> 
>> But 0 in such a context isn't an integer, it's a pointer...
>
> No it's not.

I don't have a copy of the standard handy, but google shows this snippet on
the info-minux mailing list:

   From ANSI X3.159-1989 3.2.2.3:

      An integral constant expression with the value 0, or such an expression
      cast to the type void *, is called a null pointer constant. If a null
      pointer constant is assigned to or compared for equality to a pointer,
      the constant is converted to a pointer of that type.

> Final word: K&R C without prototypes etc is still "legal C". That doesn't
> make it legal kernel code.

Your prerogative.

-Miles
-- 
"1971 pickup truck; will trade for guns"

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Linus Torvalds]

On Thu, 8 Jul 2004, Miles Bader wrote:
> >
> > No it's not.
> 
> I don't have a copy of the standard handy

I have one. It's in my head. It's called the Linux Kernel C standard. Some 
of it is documented in CodinggStyle, others is just codified in existing 
practice.

> but google shows this snippet on the info-minux mailing list:
> 
>    From ANSI X3.159-1989 3.2.2.3:

That's some totally irrelevant standard that only acts as a rough 
guideline. It dosn't know about inline assembly, and it doesn't know about 
coding standards, and it allows the most atrocious code. There's even a 
contest in making such code - it's called the Obfuscated C Contest, and it 
actually encourages using strict ANSI rules.

Me, I don't accept the kind of entries the OCC accepts. 

		Linus

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Dave Jones]

On Thu, Jul 08, 2004 at 08:58:18AM -0700, Linus Torvalds wrote:

 > Me, I don't accept the kind of entries the OCC accepts. 

drivers/char/drm/ disagrees 8-)

SCNR

		Dave

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Geert Uytterhoeven]

On Thu, 8 Jul 2004, Dave Jones wrote:
> On Thu, Jul 08, 2004 at 08:58:18AM -0700, Linus Torvalds wrote:
>
>  > Me, I don't accept the kind of entries the OCC accepts.
>
> drivers/char/drm/ disagrees 8-)

In that case... Linus, please remove those drivers, unless someone (DaveJ?)
converts them from OCC to LKC.

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Roman Zippel]

Hi,

On Thu, 8 Jul 2004, Linus Torvalds wrote:

> I have one. It's in my head. It's called the Linux Kernel C standard. Some 
> of it is documented in CodinggStyle, others is just codified in existing 
> practice.

So far we have been quite liberal in style questions, what annoys me here 
is that people send warning patches directly to you without even notifying 
the maintainers. If you want people to conform people to a certain 
CodingStyle please document officially in the kernel, sparse isn't 
distributed with the kernel and the sparse police is silently changing the 
kernel all over the place with sometimes questionable benefit. Only the 
__user warnings had really found the bugs, but the rest I've seen changes 
perfectly legal code.

bye, Roman

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Geert Uytterhoeven]

On Fri, 9 Jul 2004, Roman Zippel wrote:
> On Thu, 8 Jul 2004, Linus Torvalds wrote:
> > I have one. It's in my head. It's called the Linux Kernel C standard. Some
> > of it is documented in CodinggStyle, others is just codified in existing
> > practice.
>
> So far we have been quite liberal in style questions, what annoys me here
> is that people send warning patches directly to you without even notifying
> the maintainers. If you want people to conform people to a certain
> CodingStyle please document officially in the kernel, sparse isn't
> distributed with the kernel and the sparse police is silently changing the
> kernel all over the place with sometimes questionable benefit. Only the

I agree, when you're talking about the `if ((x = f())' cases. We already added
the extra parentheses to shut up gcc...

> __user warnings had really found the bugs, but the rest I've seen changes
> perfectly legal code.

Sparse also found the following for me:
  - #if NEVER_DEFINED_DEFINE
  - `return f();' in a function returning void (where f() returns void as well)
  - `retval k_and_r_func(/* missing void */) { ... }'
  - `extern' keywords at function definition
  - inline functions used before definition
  - const pointer assigned to non-const pointer (strange, because usually gcc
    tells me as well)
  - `if (x &= mask)' instead of `if (x & mask)'
  - floating point constants assigned to integers
  - `struct x { .field initializer };' (i.e. missing `=')

BTW, the hardest part is adding the __user annotations and making sure
<asm/uaccess.h> does it right (speaking about macros).

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Geert Uytterhoeven]

On Sat, 10 Jul 2004, Geert Uytterhoeven wrote:
> On Fri, 9 Jul 2004, Roman Zippel wrote:
> > On Thu, 8 Jul 2004, Linus Torvalds wrote:
> > > I have one. It's in my head. It's called the Linux Kernel C standard. Some
> > > of it is documented in CodinggStyle, others is just codified in existing
> > > practice.
> >
> > So far we have been quite liberal in style questions, what annoys me here
> > is that people send warning patches directly to you without even notifying
> > the maintainers. If you want people to conform people to a certain
> > CodingStyle please document officially in the kernel, sparse isn't
> > distributed with the kernel and the sparse police is silently changing the
> > kernel all over the place with sometimes questionable benefit. Only the
>
> I agree, when you're talking about the `if ((x = f())' cases. We already added
> the extra parentheses to shut up gcc...
>
> > __user warnings had really found the bugs, but the rest I've seen changes
> > perfectly legal code.

But why does sparse complain about

    p->thread.fs = get_fs().seg;

with

    linux-m68k-2.6.7/arch/m68k/kernel/process.c:265:23: warning: expected lvalue for member dereference

? Looks valid to me?

(patch to kill the warning below, _for reference only_)

--- linux-m68k-2.6.7/arch/m68k/kernel/process.c.orig	2004-06-21 20:20:00.000000000 +0200
+++ linux-m68k-2.6.7/arch/m68k/kernel/process.c	2004-06-27 14:47:23.000000000 +0200
@@ -242,6 +242,7 @@
 	struct pt_regs * childregs;
 	struct switch_stack * childstack, *stack;
 	unsigned long stack_offset, *retp;
+	mm_segment_t fs;

 	stack_offset = THREAD_SIZE - sizeof(struct pt_regs);
 	childregs = (struct pt_regs *) ((unsigned long) (p->stack) + stack_offset);
@@ -262,7 +263,8 @@
 	 * Must save the current SFC/DFC value, NOT the value when
 	 * the parent was last descheduled - RGH  10-08-96
 	 */
-	p->thread.fs = get_fs().seg;
+	fs = get_fs();
+	p->thread.fs = fs.seg;

 	if (!FPU_IS_EMU) {
 		/* Copy the current fpu state */

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[viro]

On Sun, Jul 11, 2004 at 12:29:51PM +0200, Geert Uytterhoeven wrote:
> But why does sparse complain about
> 
>     p->thread.fs = get_fs().seg;
> 
> with
> 
>     linux-m68k-2.6.7/arch/m68k/kernel/process.c:265:23: warning: expected lvalue for member dereference
> 
> ? Looks valid to me?

It is valid and should be left alone.  sparse doesn't handle that area and
unless somebody feels heroic it will stay that way.

Note that there are very scary critters in that part of language - among
other things it contains such fun as arrays that can't be converted to
pointers in normal way, etc. (arrays that have no address, while we are
at it).

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Linus Torvalds]

On Sun, 11 Jul 2004, Geert Uytterhoeven wrote:
> 
> But why does sparse complain about
> 
>     p->thread.fs = get_fs().seg;

Sparse bug. Hey, it's not perfect, and this case is actually very easy to 
just parse, but harder to build a nice internal representation of it.

In particular, sparse internally really _should_ re-write the above as

	fnret = get_fs();
	p->thread.fs = fnret.seg;

since that is what would happen in real life, but since it never came up
in early testing, I didn't ever really get around to doing it that way.  
Oh, well.

So please don't take _all_ sparse warnings too seriously. Some of them 
literally are still due to sparse limitations. I'm happy to say that they 
are fairly few these days.

			Linus

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[H. Peter Anvin]

Followup to:  <Pine.GSO.4.58.0407102126150.10242@waterleaf.sonytel.be>
By author:    Geert Uytterhoeven <geert@linux-m68k.org>
In newsgroup: linux.dev.kernel
> 
>   - `return f();' in a function returning void (where f() returns void as well)
> 

Considering this one a bug is daft in the extreme.

Why?  Because "return f();" is the only kind of tailcall syntax C has,
and requiring that "void" functions use a different syntax is just
stupid.

Now, if the return types don't match then that's another issue.

	-hpa

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Florian Weimer]

* H. Peter Anvin:

> Followup to:  <Pine.GSO.4.58.0407102126150.10242@waterleaf.sonytel.be>
> By author:    Geert Uytterhoeven <geert@linux-m68k.org>
> In newsgroup: linux.dev.kernel
>> 
>>   - `return f();' in a function returning void (where f() returns void as well)
>> 
>
> Considering this one a bug is daft in the extreme.
>
> Why?  Because "return f();" is the only kind of tailcall syntax C has,

Huh?  If you remove the "return", it's still a valid tailcall syntax
(at least from GCC's perspective).

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[H. Peter Anvin]

Florian Weimer wrote:
> * H. Peter Anvin:
> 
> 
>>Followup to:  <Pine.GSO.4.58.0407102126150.10242@waterleaf.sonytel.be>
>>By author:    Geert Uytterhoeven <geert@linux-m68k.org>
>>In newsgroup: linux.dev.kernel
>>
>>>  - `return f();' in a function returning void (where f() returns void as well)
>>>
>>
>>Considering this one a bug is daft in the extreme.
>>
>>Why?  Because "return f();" is the only kind of tailcall syntax C has,
> 
> 
> Huh?  If you remove the "return", it's still a valid tailcall syntax
> (at least from GCC's perspective).

Yes, but as I said in the portion of the message you deleted, it means you 
have to use a different syntax with void functions as for all other functions.

That is daft in the extreme.

	-hpa

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Herbert Xu]

On Wed, Jul 07, 2004 at 10:22:32PM -0700, Linus Torvalds wrote:
>
> I'm sorry that you are such a K&R-C bigot that you don't like type 
> checking. But the kernel DOES like type checking, and the kernel is not 
> K&R C. The kernel uses strict ANSI, and in fact, is _more_ strict than 
> ANSI C is in many many ways.

Well it's your project so you get to set the coding style.

But it is ironic that you call people who use 0 in a pointer context
K&R-C bigots.  One of the principal reason why NULL exists at all
is in fact the lack of prototypes in K&R...
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Andreas Schwab]

Herbert Xu <herbert@gondor.apana.org.au> writes:

> But it is ironic that you call people who use 0 in a pointer context
> K&R-C bigots.  One of the principal reason why NULL exists at all
> is in fact the lack of prototypes in K&R...

There is one place where even prototypes won't help, which is varargs
functions like execl.  But I don't think the kernel uses functions with
execl-like argument lists.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Herbert Xu]

On Thu, Jul 08, 2004 at 03:10:11PM +0200, Andreas Schwab wrote:
> 
> There is one place where even prototypes won't help, which is varargs
> functions like execl.  But I don't think the kernel uses functions with
> execl-like argument lists.

Actually printk is variadic.  But gcc will provide warnings if it
sees a mismatch between the format and the arguments.
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Andreas Schwab]

Herbert Xu <herbert@gondor.apana.org.au> writes:

> On Thu, Jul 08, 2004 at 03:10:11PM +0200, Andreas Schwab wrote:
>> 
>> There is one place where even prototypes won't help, which is varargs
>> functions like execl.  But I don't think the kernel uses functions with
>> execl-like argument lists.
>
> Actually printk is variadic.

There are quite a few other varargs functions in the kernel, but I was
specifically thinking of the use of a null pointer for terminating the
argument list like execl does.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Linus Torvalds]

On Thu, 8 Jul 2004, Herbert Xu wrote:
>
> Chris Wright <chrisw@osdl.org> wrote:
> > Fixup another round of sparse warnings of the type:
> >        warning: Using plain integer as NULL pointer
> 
> What's wrong with using 0 as the NULL pointer? In contexts where
> a plain 0 is unsafe, NULL is usually unsafe as well.

It's not about "unsafe". It's about being WRONG.

The fact is, people who write "0" are either living in the stone age, or 
are not sure about the type. "0" is an _integer_. It's not a pointer. It 
may be legal C, but that doesn't make it right anyway. "0" also happens to 
be one of the more _common_ integers, so mistakes happen.

Looking at the code, people that used "0" for NULL pointers quite often 
obviously were NOT aware of the types. The code just happened to pass 
through the compiler without warnings.

The same is true the other way too. I've seen too many damn people who use 
NULL in an integer context, and any compiler system that makes NULL be 
just a plain "0" is frigging _broken_.  NULL is _not_ an integer. Never 
has been, and if the compiler doesn't warn loudly about obviously idiotic 
code, then the compiler is broken.

In other words:

	char * p = 0;	/* IS WRONG! DAMMIT! */
	int i = NULL;	/* THIS IS WRONG TOO! */

and anybody who writes code like the above either needs to get out of the 
kernel, or needs to get transported to the 21st century. 

		Linus

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Timothy Miller]

Linus Torvalds wrote:

> It's not about "unsafe". It's about being WRONG.
> 
> The fact is, people who write "0" are either living in the stone age, or 
> are not sure about the type. "0" is an _integer_. It's not a pointer. It 
> may be legal C, but that doesn't make it right anyway. "0" also happens to 
> be one of the more _common_ integers, so mistakes happen.

Not to be picky, and I realize that we're not using C++ here, and it may 
not apply, but every C++ text I've read deprecates NULL and says to use 
0.  That is, THE WAY that you specify a null pointer in C++ is with a 
zero.  It's no surprise that C programmers might pick up that habit.

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Linus Torvalds]

On Thu, 8 Jul 2004, Timothy Miller wrote:
> 
> Not to be picky, and I realize that we're not using C++ here, and it may 
> not apply, but every C++ text I've read deprecates NULL and says to use 
> 0.  That is, THE WAY that you specify a null pointer in C++ is with a 
> zero.  It's no surprise that C programmers might pick up that habit.

It's a bad habit, and it's one (of many) bad things C++ does. The sad part 
is that C++ does it for all the wrong reasons - there's no reason to not 
realize that ((void *)0) is an even better special case, and indeed last I 
heard at least gcc does allow that in C++ too.

The fact is, when somebody else picks up a mistake, that doesn't make it 
any less of a mistake. And it's not like C++ is the paragon of good taste 
anyway.

I've seen too damn many people mistake NULL and NUL (admit it, you've seen 
it too), and I've seen code like

	char c = NULL;

and any system where that goes through without a warning is totally 
broken. And yes, that includes a _lot_ of C++ braindamages.

		Linus

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Linus Torvalds]

On Thu, 8 Jul 2004, Linus Torvalds wrote:
> 
> The fact is, when somebody else picks up a mistake, that doesn't make it 
> any less of a mistake.

Whee. Five seconds after writing the above and sending it off, what do I 
see on CNN but:

	Another 'Police Academy' in works
	New film would be eighth in series

	LOS ANGELES, California (Hollywood Reporter) -- "Police Academy"
	is back.  After a decade's absence from the big screen, the cop
	comedy franchise is gearing up for an eighth installment.

I rest my case.

		Linus

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Timothy Miller]

Linus Torvalds wrote:

> 
> I've seen too damn many people mistake NULL and NUL (admit it, you've seen 
> it too), and I've seen code like
> 
> 	char c = NULL;

THIS is simply a case of the programmer not understanding what NULL 
means.  When I use '0' for a pointer, I know EXACTLY what I mean, and I 
also know when '0' might be ambiguous, and when I don't know what I'm 
allowed to do, then I play it REALLY safe and typecast 0 to exactly the 
pointer type I need.

I suppose it's good form to use the safest syntax in all cases.  Good 
for readability for people who need more redundancy to read the code.

Perhaps the Linux kernel should have a convention where all NULL 
pointers are typecast explicitly.  I can hear the cries of pain already.  :)

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Timothy Miller]

Denis Vlasenko wrote:
> On Thursday 08 July 2004 19:52, Timothy Miller wrote:
> 
>>Linus Torvalds wrote:
>>
>>>I've seen too damn many people mistake NULL and NUL (admit it, you've
>>>seen it too), and I've seen code like
>>>
>>>	char c = NULL;
>>
>>THIS is simply a case of the programmer not understanding what NULL
>>means.  When I use '0' for a pointer, I know EXACTLY what I mean, and I
>>also know when '0' might be ambiguous, and when I don't know what I'm
>>allowed to do, then I play it REALLY safe and typecast 0 to exactly the
>>pointer type I need.
> 
> 
> The question is, whether readers of your code (including compiler)
> will be able to be sure that there is no error in
> 
> 	f(a,b,c,d,e,0,f,g,h);
> 
> statement or not. Better typecheck that 0.

This I agree with, definately.  It's very important to make your code 
readable, and if it's not obvious from context, make it obvious.  Cases 
like the above are one of the reasons I like languages like Verilog 
where you can pass parameters by specifying the parameter name.

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Andreas Schwab]

Timothy Miller <miller@techsource.com> writes:

> Denis Vlasenko wrote:
>> The question is, whether readers of your code (including compiler)
>> will be able to be sure that there is no error in
>> 	f(a,b,c,d,e,0,f,g,h);
>> statement or not. Better typecheck that 0.
>
> This I agree with, definately.  It's very important to make your code
> readable, and if it's not obvious from context, make it obvious.  Cases
> like the above are one of the reasons I like languages like Verilog where
> you can pass parameters by specifying the parameter name.

If your function needs nine arguments it is not readable by
definition. :-)

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Andrew Pimlott]

On Mon, Jul 12, 2004 at 06:12:23PM +0200, Andreas Schwab wrote:
> If your function needs nine arguments it is not readable by
> definition. :-)

"If you have a procedure with ten parameters, you probably missed some."
- Alan Perlis

Andrew

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Timothy Miller]

Andreas Schwab wrote:
> Timothy Miller <miller@techsource.com> writes:
> 
> 
>>Denis Vlasenko wrote:
>>
>>>The question is, whether readers of your code (including compiler)
>>>will be able to be sure that there is no error in
>>>	f(a,b,c,d,e,0,f,g,h);
>>>statement or not. Better typecheck that 0.
>>
>>This I agree with, definately.  It's very important to make your code
>>readable, and if it's not obvious from context, make it obvious.  Cases
>>like the above are one of the reasons I like languages like Verilog where
>>you can pass parameters by specifying the parameter name.
> 
> 
> If your function needs nine arguments it is not readable by
> definition. :-)


Yeah... unfortunately, in chip design, you can't really avoid that. 
VHDL has structures which allow you to aggregate different signals, but 
Verilog doesn't have that feature, so you end up with hundreds of module 
parameters sometimes.  That's when pass-by-name really pays off.

But then again, everything you know about software development is 
completely goes out the window when doing chip design.  (i.e. in C, less 
code usually produces better results.  The opposite is true in Verilog.)

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Eric W. Biederman]

Linus Torvalds <torvalds@osdl.org> writes:

> On Thu, 8 Jul 2004, Herbert Xu wrote:
> >
> > Chris Wright <chrisw@osdl.org> wrote:
> > > Fixup another round of sparse warnings of the type:
> > >        warning: Using plain integer as NULL pointer
> > 
> > What's wrong with using 0 as the NULL pointer? In contexts where
> > a plain 0 is unsafe, NULL is usually unsafe as well.
> 
> It's not about "unsafe". It's about being WRONG.

Does this mean constructs like:
``if (pointer)'' and ``if (!pointer)'' are also outlawed.

And do we then need to initialize static pointers to NULL instead
of letting them be implicitly 0.

Is doing memset(&(struct with_embeded_pointers), 0, sizeof(struct))
also wrong?

I don't see that 0 is WRONG.  I do agree that ``((void *)0)'' is
slightly more typesafe than ``0'', but since we don't have a lot of
(void *) pointers in the kernel that is still the WRONG pointer type.

I do see that NULL has superior readability and maintainability and so
should be encouraged by Documentation/CodingStyle.

The B and K&R roots of a simple single type language are what give C
most of it's simplicity flexibility and power.  Please don't be so
eager to throw those out.  

You want to be so typesafe it sounds like you want to recode the
kernel in Pascal.  You've written sparse, so it should be just a little
more work to write a Pascal backend.  After that the kernel will be so
typesafe the compiler won't let us poor programmers get it wrong.

Eric

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Paul Jackson]

Eric wrote:
> 
> Does this mean constructs like:
> ``if (pointer)'' and ``if (!pointer)'' are also outlawed.
> 
> And do we then need to initialize static pointers to NULL instead
> of letting them be implicitly 0.
> 
> Is doing memset(&(struct with_embeded_pointers), 0, sizeof(struct))
> also wrong?

I suspect not.  Up to Linus.  This is all about writing code that
doesn't bite.

Since mostly it's us humans doing the writing, this is more a human
engineering problem than a pure mathematics problem such as Dijkstra
or Wirth were closer to addressing.

Let someone with demonstrated good taste dictate the style choices
that lead to short, sweat, but seldom screwy code.

It's all arbitrary as hell.  The proof is in the pudding.

-- 
                          I won't rest till it's the best ...
                          Programmer, Linux Scalability
                          Paul Jackson <pj@sgi.com> 1.650.933.1373

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Kyle Moffett]

On Jul 09, 2004, at 19:49, Paul Jackson wrote:
> I suspect not.  Up to Linus.  This is all about writing code that
> doesn't bite.
>
> Since mostly it's us humans doing the writing, this is more a human
> engineering problem than a pure mathematics problem such as Dijkstra
> or Wirth were closer to addressing.
>
> Let someone with demonstrated good taste dictate the style choices
> that lead to short, sweat, but seldom screwy code.
>
> It's all arbitrary as hell.  The proof is in the pudding.

The place this probably matters most is in various function calls. For
example, given the following prototype, (and ignoring the fact that
gcc is generally able to properly compile all of these), which is the
most clear?  These are all "logically" correct, for the most part, but
as humans we have certain readability standards.

int some_function(int a, void *b, char *c, unsigned char d, int e);

A)	int res = some_function(0,0,0,0,0);
B)	int res = some_function(NULL,NULL,NULL,NULL,NULL);
C)	int res = some_function(0,NULL,NULL,'\0',0);

C is the most expressive of the intent of the code, and the least
likely to contain bugs.

Cheers,
Kyle Moffett

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Herbert Xu]

On Fri, Jul 09, 2004 at 09:43:18PM -0400, Kyle Moffett wrote:
>
> most clear?  These are all "logically" correct, for the most part, but
> as humans we have certain readability standards.

Nope, B is undefined.

> int some_function(int a, void *b, char *c, unsigned char d, int e);
> 
> A)	int res = some_function(0,0,0,0,0);
> B)	int res = some_function(NULL,NULL,NULL,NULL,NULL);
> C)	int res = some_function(0,NULL,NULL,'\0',0);
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Alexandre Oliva]

On Jul  9, 2004, Herbert Xu <herbert@gondor.apana.org.au> wrote:

> On Fri, Jul 09, 2004 at 09:43:18PM -0400, Kyle Moffett wrote:
>> 
>> most clear?  These are all "logically" correct, for the most part, but
>> as humans we have certain readability standards.

> Nope, B is undefined.

Nope, B is implementation-defined.  The conversion from pointers to
integers is implementation-defined, and it's meant to be unsurprising
to those familiar with the architecture.  I.e., if you can
zero-initialize a pointer and get a NULL pointer back, it's quite
likely that a NULL pointer will convert back to (int)0, even though
it's not required by the C Standard AFAICT.

>> int some_function(int a, void *b, char *c, unsigned char d, int e);

>> B)	int res = some_function(NULL,NULL,NULL,NULL,NULL);

-- 
Alexandre Oliva             http://www.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer   aoliva@{redhat.com, gcc.gnu.org}
Free Software Evangelist  oliva@{lsd.ic.unicamp.br, gnu.org}

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Linus Torvalds]

On Fri, 9 Jul 2004, Eric W. Biederman wrote:
>
> Does this mean constructs like:
> ``if (pointer)'' and ``if (!pointer)'' are also outlawed.

Of course not.

Why should they be?

What's considered bad form is:
 - assignments in boolean context (because of the confusion of "=" and 
   "==")
 - thinking the constant "0" is a pointer.

There's no reason why "if (!ptr)" would be wrong. That has zero confusion 
about 0 vs NULL.

The confusion about "0" is that in traditional C it means two things: it 
can either be an integer (the common case) or it can sometimes be a 
pointer. That kind of semantic confusion is bad.

But it has nothing to do with the _value_ zero, or testing pointers for
being non-NULL. The value zero is not about semantic confusion, it's just 
a bit pattern. And testing pointers is not ambiguous: when you test a 
pointer, it's _un_ambigiously checking that pointer for NULL.

Problems arise when there is room for confusion, and that's when the 
compiler should (and does) warn. If something is unambiguous, it's not 
bad.

		Linus

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Linus Torvalds]

On Fri, 9 Jul 2004, Linus Torvalds wrote:
> 
> Problems arise when there is room for confusion, and that's when the 
> compiler should (and does) warn. If something is unambiguous, it's not 
> bad.

Btw, has anybody who is complaining about the 0/NULL fixing actually 
_looked_ at the code?

Every single time a 0 was replaced by a NULL it was an obvious
_improvement_ to the code. Not just "once". EVERY SINGLE TIME. It might be
irritating to see the patches, and there might be too many of them since
nothing has ever automatically noticed the bugs until now, but the fact
is, there is not even any gray areas here - if you look at any of the
patches being applied, they are ALL clearly making things more readable in
their local context.

I really don't see the point of complaining about the fixes. There's just
_no_ way to say that "0" is more readable than "NULL" in any of the cases.  
I dare you - show _one_ case where a 0/NULL patch was wrong or even
remotely debatable. I dare you.

		Linus

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Andrea Arcangeli]

On Fri, Jul 09, 2004 at 11:23:52PM -0700, Linus Torvalds wrote:
> I really don't see the point of complaining about the fixes. There's just
> _no_ way to say that "0" is more readable than "NULL" in any of the cases.  
> I dare you - show _one_ case where a 0/NULL patch was wrong or even
> remotely debatable. I dare you.

I definitely agree.

Several years ago I once wrote a singificant piece of code for a projet
with #define NULL -1UL, this actually wasn't my choice but a requirement
of the project (the headers were pre-defined) but it worked perfectly
since we never did '!ptr' we always did 'ptr == NULL' instead (etc..).
So at runtime it has never been a problem because we coded with NULL !=
0 in mind. Of course I known normally NULL is always equal to 0 but I
didn't realize that defining NULL !=0 wasn't exactly the C language (I
learnt it later on the hard way in some mailing list, I believe at some
point I did patches like the one in this thread but claiming it to be a
bugfix, and not just a cleanup ;).

IIRC my argument about these patches being bugfixes, was about an
architecture with a valid page mapped at address 0, that wouldn't
generate a segfault. This is incidentally why we had to use NULL = -1
instead of NULL = 0. The answer I got at that time form some C guru is
that I would need to hack the compiler specifically for such achitecture
to accomodate for NULL = -1, so that '!ptr' will be the same as 'ptr ==
-1UL' (for pointers). In practice I think it has been a lot easier for
us to avoid using '!ptr' than to hack gcc... 

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Eric W. Biederman]

Andrea Arcangeli <andrea@suse.de> writes:

> IIRC my argument about these patches being bugfixes, was about an
> architecture with a valid page mapped at address 0, that wouldn't
> generate a segfault. This is incidentally why we had to use NULL = -1
> instead of NULL = 0. The answer I got at that time form some C guru is
> that I would need to hack the compiler specifically for such achitecture
> to accomodate for NULL = -1, so that '!ptr' will be the same as 'ptr ==
> -1UL' (for pointers). In practice I think it has been a lot easier for
> us to avoid using '!ptr' than to hack gcc... 

Well gcc is not terribly hackable that way, something about decades
of code bloat...

The thing to keep in mind is that it was the integer to pointer
conversion that needed to be hacked.  (void *)0 is the null pointer
constant by definition.  So you it is technically illegal in that
context to use (ptr *)0 to refer to the first page of memory.  It
must be another number that you can convert and manipulate.

For a single program I do agree that it probably would not be worth it
but if you were delivering an architecture where people wrote lots
of code it would be better fix the compiler to be standards conforming.

Eric

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Roland Dreier]

    Linus> I really don't see the point of complaining about the
    Linus> fixes. There's just _no_ way to say that "0" is more
    Linus> readable than "NULL" in any of the cases.  I dare you -
    Linus> show _one_ case where a 0/NULL patch was wrong or even
    Linus> remotely debatable. I dare you.

I don't know if any of the 0/NULL kernel patches were of this form,
but I've seen sparse complain about this in my code and found it
somewhat annoying.  I think the following is at least remotely debatable...

Suppose I have

	struct foo {
		int a;
		int b;
	};

then sparse is perfectly happy with someone clearing out a struct foo
like this:

	struct foo bar = { 0 };

but then if someone changes struct foo to be

	struct foo {
		void *x;
		int a;
		int b;
	};

sparse will complain about that initialization, and all of the fixes
I can think of seem somewhat worse than the original to me:

	struct foo bar = { NULL };   /* will I have to change this
                                        again if struct foo changes? */

or

	struct foo bar = { .a = 0 }; /* why do I have to name a member? */

or

	struct foo bar;
	memset(&bar, 0, sizeof bar); /* WRONG if a null pointer is not
                                        the bit pattern 0 */

 - Roland

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Eyal Lebedinsky]

Roland Dreier wrote:
> Suppose I have
> 
> 	struct foo {
> 		int a;
> 		int b;
> 	};
> 
> then sparse is perfectly happy with someone clearing out a struct foo
> like this:
> 
> 	struct foo bar = { 0 };
> 
> but then if someone changes struct foo to be
> 
> 	struct foo {
> 		void *x;
> 		int a;
> 		int b;
> 	};
> 
> sparse will complain about that initialization, and all of the fixes
> I can think of seem somewhat worse than the original to me:

Come on, this is madness. By accident, the first memeber which
changed from 'int' to 'void *' now accepts the old initializer.
In my book this is a really bad thing because you just changed
the semantics of the initialiser '{ 0 }' quietly.

BTW, if nothing else, don't add new members at the top.

--
Eyal Lebedinsky		(eyal@eyal.emu.id.au)

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Olaf Titz]

> 	struct foo {
> 		int a;
> 		int b;
> 	};
>
> then sparse is perfectly happy with someone clearing out a struct foo
> like this:
>
> 	struct foo bar = { 0 };
>
> but then if someone changes struct foo to be
>
> 	struct foo {
> 		void *x;
> 		int a;
> 		int b;
> 	};
>
> sparse will complain about that initialization, and all of the fixes

It complains rather rightly. Think what happens if the original
initializer was
   struct foo bar = { 1 };

This ambiguity may well be the main reason for the C99 initializer
syntax.

Olaf

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Eric W. Biederman]

Linus Torvalds <torvalds@osdl.org> writes:

> On Fri, 9 Jul 2004, Eric W. Biederman wrote:
> >
> > Does this mean constructs like:
> > ``if (pointer)'' and ``if (!pointer)'' are also outlawed.
> 
> Of course not.
> 
> Why should they be?

Only because the definition of the semantics of ``if'' is in terms of
comparisons with ``0'',  and I am familiar enough with the C
programming language that, that is how I read it.  It is still 
the case that because the comparison happens in pointer context the
``0'' referred  to is the null pointer constant.  

For some of us who are extremely familiar with C your argument is
confusing.  You make statements that sound like they are about the
definition of the C programming language when in fact they are
criticism of a given C programming style.  

Since I am already making distinctions 0 as the integer value and
0 as the pointer constant when 0 is implicitly introduced.  It is
really not confusing to me in the case of manifest constants.

> What's considered bad form is:
>  - assignments in boolean context (because of the confusion of "=" and 
>    "==")
>  - thinking the constant "0" is a pointer.

I would agree that using the constant "0" in a pointer context
when a more explicit NULL is bad form.  But "0" is the one
legal way in C to write the NULL pointer constant.

> There's no reason why "if (!ptr)" would be wrong. That has zero confusion 
> about 0 vs NULL.

For me it has exactly the same level of confusion as the cases that
are being fixed has.  I have to know the type to know if it is testing
against the NULL pointer constant or if it is testing against the
integer value zero.
 
> The confusion about "0" is that in traditional C it means two things: it 
> can either be an integer (the common case) or it can sometimes be a 
> pointer. That kind of semantic confusion is bad.

Either that or it can be called greater expressive power though fewer
concepts. 

I like the fact this allows cases like ``if (!ptr)'' and friends.
 
> But it has nothing to do with the _value_ zero, or testing pointers for
> being non-NULL. The value zero is not about semantic confusion, it's just 
> a bit pattern. And testing pointers is not ambiguous: when you test a 
> pointer, it's _un_ambigiously checking that pointer for NULL.

see above.

> Problems arise when there is room for confusion, and that's when the 
> compiler should (and does) warn. If something is unambiguous, it's not 
> bad.

The compiler is compiling the correct code so the code is clearly not
ambiguous.  But since types are not always obvious to a person
staring at the code using the more explicit form of the constant
i.e. NULL or '\0' instead of 0 adds useful redundancy.

Hopefully that explains why I objected to the way you can out against
using ``0'' as the null pointer constant.

Eric

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Herbert Xu]

On Sat, Jul 10, 2004 at 03:39:13AM -0600, Eric W. Biederman wrote:
> 
> I would agree that using the constant "0" in a pointer context
> when a more explicit NULL is bad form.  But "0" is the one
> legal way in C to write the NULL pointer constant.

One reason I dislike the push to use NULL everywhere is that
new-comers may feel a false sense of security when using NULL.

This will bite in places where an explicit cast is needed to turn
NULL into a null pointer of the correct type, i.e., pointer arguments
to variadic functions.

A neat trick to catch such errors is to define NULL to be 0LL.
This is perfectly legal under ANSI and will generate a different
representation on i386.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Olaf Titz]

> Only because the definition of the semantics of ``if'' is in terms of
> comparisons with ``0'',  and I am familiar enough with the C
> programming language that, that is how I read it.  It is still
> the case that because the comparison happens in pointer context the
> ``0'' referred  to is the null pointer constant.
>
> For some of us who are extremely familiar with C your argument is
> confusing.  You make statements that sound like they are about the
> definition of the C programming language when in fact they are
> criticism of a given C programming style.
>
> Since I am already making distinctions 0 as the integer value and
> 0 as the pointer constant when 0 is implicitly introduced.  It is
> really not confusing to me in the case of manifest constants.

So the real question is why C has no "null" token like Java or Pascal
and re-uses the "0" token (which is really no token by itself but a
numeric-constant token which happens to have a special value).

If your argument holds that "0" in a pointer context really is a
special token like Java's "null" (which is explicitly defined by the
standard as a pointer different from any other pointer) then it would
be possible to implement a compiler which not only defines NULL to
-1L, as someone mentioned here, but actually generates an all-ones bit
pattern out of the constant 0 when used in a pointer context, yet
generates an all-zeros bit pattern when used in an integer context.
It also would have to implement the implicit null-comparison in a
boolean context appropriately.

This probably would work with all programs which make a clear
distinction between pointers and integer values, but you have to be
really pedantic about these "stylistic" issues to always get it right
in C. (Worse in C++ where usage of NULL is discouraged, I've always
wondered about the reasons.)

The real problem, however, is that this "stylistic" issue may quickly
become a _correctness_ issue as soon as the actual bit pattern of a
pointer in memory is taken to have any meaning. I.e. it already starts
when you initialize a structure with memset().

And this is the reason why really strongly typed languages never allow
assignment of a pointer to or from any other data type. (Java is that
strict, Pascal has a possible backdoor in the "record case" structure
if implemented as overlays like C's unions but otherwise is as strict
too, not sure about Ada.)
_The bit pattern of a pointer must have no meaning to the program_.

To answer the question from the first paragraph, it is "because C does
_not_ strongly distinguish between pointer and non-pointer values".
And for this reason people have invented the NULL constant, and the
convention that
    "if (x)" means "if (x != 0)" in numeric context, and
    "if (x)" means "if (x != NULL)" in pointer context.

This resolves all the ambiguities and allows people to use C as a
strongly typed language, but can break with program(mer)s taking
pointers as equivalent to numbers.

Olaf

PS. I wonder how many bugs have been avoided in the Linux kernel by
this kind of style pedantery vs. how many bugs have crept into other
systems where people are more sloppy.

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Geert Uytterhoeven]

On Sun, 11 Jul 2004, Olaf Titz wrote:
> in C. (Worse in C++ where usage of NULL is discouraged, I've always
> wondered about the reasons.)

[ wondered about this as well, but the answer has been posted before in this
  thread ]

Because C++ doesn't do implicit conversions from void * to anything *.

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Alexandre Oliva]

On Jul 13, 2004, Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> On Sun, 11 Jul 2004, Olaf Titz wrote:
>> in C. (Worse in C++ where usage of NULL is discouraged, I've always
>> wondered about the reasons.)

> [ wondered about this as well, but the answer has been posted before in this
>   thread ]

> Because C++ doesn't do implicit conversions from void * to anything *.

So what?  NULL must have an integral type in C++.  void* is explicitly
forbidding in the C++ Standard.

I don't see that NULL is discouraged in C++.  It's mostly redundant,
like it is in C, and it's not safe for varargs even on machines where
NULL pointers are represented can be zero-initialized, because
pointers to members and regular pointers don't even have the same
size, unlike C, that doesn't have pointers to members and thus can
safely use (intptr_t)0 for NULL and it will even work for varargs
(given the considerations above).

-- 
Alexandre Oliva             http://www.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer   aoliva@{redhat.com, gcc.gnu.org}
Free Software Evangelist  oliva@{lsd.ic.unicamp.br, gnu.org}

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Elladan]

On Sun, Jul 11, 2004 at 11:47:58PM +0200, Olaf Titz wrote:
> > For some of us who are extremely familiar with C your argument is
> > confusing.  You make statements that sound like they are about the
> > definition of the C programming language when in fact they are
> > criticism of a given C programming style.
> >
> > Since I am already making distinctions 0 as the integer value and
> > 0 as the pointer constant when 0 is implicitly introduced.  It is
> > really not confusing to me in the case of manifest constants.
> 
> So the real question is why C has no "null" token like Java or Pascal
> and re-uses the "0" token (which is really no token by itself but a
> numeric-constant token which happens to have a special value).
> 
> If your argument holds that "0" in a pointer context really is a
> special token like Java's "null" (which is explicitly defined by the
> standard as a pointer different from any other pointer) then it would
> be possible to implement a compiler which not only defines NULL to
> -1L, as someone mentioned here, but actually generates an all-ones bit
> pattern out of the constant 0 when used in a pointer context, yet
> generates an all-zeros bit pattern when used in an integer context.
> It also would have to implement the implicit null-comparison in a
> boolean context appropriately.

This indeed is possible, and has been implemented in the past.  Some
segmented architectures use different segments for different data types,
and a segment tag is part of the appropriate null pointer.  Also,
obviously, some pointer types are different widths on some
architectures.

The C languages was defined in such a way that it works fine on such an
architecture.  int *p = 0;  will always result in the appropriate null
pointer for integers.

The one exception is if you have a variadic function, in which case you
have to cast the null pointer to the right type, eg. 
printf("%s\n", (char*)0);

What's not allowed in conformant portable C is clearing pointers to null
using memset.  That just results in a zero bit pattern.

Eg.,

struct foo {
	int *bar;
} x;
memset(&x, 0, sizeof(x));

However, implicit initializers are valid, and will fill in pointer types
with the appropriate null value, eg:

struct foo {
	int *bar;
} x = {};

In addition, defining NULL as (void*)0 is completely broken.  A void*
null pointer is not valid as a null pointer for some other type - void*
is a transitional type, it simply holds the bit values of other pointers
so you can cast them back later.

-J

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Alexandre Oliva]

On Jul 11, 2004, Olaf Titz <olaf@bigred.inka.de> wrote:

> (Worse in C++ where usage of NULL is discouraged, I've always
> wondered about the reasons.)

FWIW, g++ defines NULL to the __null GCC-extension keyword, that
issues a warning if it's doesn't decay to a pointer type, but behaves
like the integral constant of value 0 otherwise, as required by the
C++ standard.

I suppose Linux might benefit from such an extension implemented in C
as well.  Well, looks like sparse already takes care of that, and GCC
has entered an new-extension-avoidance mode (or mood? :-) a few years
ago.

-- 
Alexandre Oliva             http://www.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer   aoliva@{redhat.com, gcc.gnu.org}
Free Software Evangelist  oliva@{lsd.ic.unicamp.br, gnu.org}

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Roman Zippel]

Hi,

On Fri, 9 Jul 2004, Linus Torvalds wrote:

> What's considered bad form is:
>  - assignments in boolean context (because of the confusion of "=" and 
>    "==")

gcc already warns about this, what value has this extra stuff?

bye, Roman

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Alexandre Oliva]

On Jul 10, 2004, Linus Torvalds <torvalds@osdl.org> wrote:

> On Fri, 9 Jul 2004, Eric W. Biederman wrote:
>> 
>> Does this mean constructs like:
>> ``if (pointer)'' and ``if (!pointer)'' are also outlawed.

> Of course not.

> Why should they be?

Err...  Because the conditional expression is implicitly compared with
0 [6.8.4.1]/#2.  If 0 is not to be used explicitly in pointer
contexts, why should it be ok to use it implicitly?

> What's considered bad form is:
[snip]
>  - thinking the constant "0" is a pointer.

> There's no reason why "if (!ptr)" would be wrong.

[6.5.3.3]/#5 defines the result of the logical negation operator
based on the result of comparing the expression with 0.

> But it has nothing to do with the _value_ zero, or testing pointers for
> being non-NULL. The value zero is not about semantic confusion, it's just 
> a bit pattern. And testing pointers is not ambiguous: when you test a 
> pointer, it's _un_ambigiously checking that pointer for NULL.

I don't see why (!ptr) is any more confusing than (ptr != 0), and why
(ptr != NULL) would be any clearer.  Is `ptr != 0' one of the cases
that are not bad?

-- 
Alexandre Oliva             http://www.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer   aoliva@{redhat.com, gcc.gnu.org}
Free Software Evangelist  oliva@{lsd.ic.unicamp.br, gnu.org}

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Linus Torvalds]

On Sat, 10 Jul 2004, Alexandre Oliva wrote:
> 
> > Why should they be?
> 
> Err...  Because the conditional expression is implicitly compared with
> 0 [6.8.4.1]/#2.  If 0 is not to be used explicitly in pointer
> contexts, why should it be ok to use it implicitly?

Exactly BECAUSE it's not an explicitly WRONG type of 0.

"0" is a number token. It is totally illogical from a conceptual 
standpoint to use it as a pointer. It makes no sense from any syntactic 
standpoint, and it's very much an ugly special case because K&R didn't 
want to add a keyword for NULL.

But the fact is, even early on, exactly _because_ "0" is illogical as a 
pointer, K&R added a

	#define NULL 0

to make it make _syntactic_ sense to use "NULL" as a pointer, even though 
the language lacked the specific keyword. So from a _syntactic_ 
standpoint, NULL is a pointer, even if from an implementation standpoint 
NULL ended up being this totally illogical integer 0.

In contrast, there is nothing syntactically strange about comparing a 
non-boolean (even though Pascal and other languages make it illegal). So 
here again, the "compare against 0" is an _implementation_ issue, not a 
conceptual syntactic confusion.

What I object to in using "0" as a pointer is that it changes the meaning 
of the token "0" depending on semantic information that may not even be 
very local. In contrast

	if (ptr)
		..

has no such confusion.

> [6.5.3.3]/#5 defines the result of the logical negation operator
> based on the result of comparing the expression with 0.

And you're totally confusing the "this is defined to be equivalent" as an 
implementation standpoint with "it's the same thing".

For example, the code

	5[ptr]

is _defined_ to be exactly the same as

	ptr[5]

since they both really mean

	*(ptr + 5)

and nothing else. HOWEVER, despite the fact that the C language _defines_ 
that they are exactly equivalent, I claim that anybody who writes "5[ptr]" 
is an ass.

For that same reason, your argument is totally irrelevant. Yes, 

	if (ptr)

is _defined_ to be exactly the same as

	if (ptr != 0)

but I claim that anybody who writes the latter is an ass, because it makes 
no syntactic sense. The same way "5[ptr]" doesn't make syntactic sense, 
even though the compiler will silently accept it.

Got it?

		Linus

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Eyal Lebedinsky]

Executive summary:
	It is not about the law (ANSI). It is about best practices
	that improve code quality. And transparency in the
	programmer's expression should always be welcome.

This thread can easily degrade into a religious argument
(an oxymoron, can't argue about religion so just don't do
it). I hope the below addresses the issues raised here
and gives reason for my opinions.

Eric W. Biederman wrote:
> Does this mean constructs like:
> ``if (pointer)'' and ``if (!pointer)'' are also outlawed.

Very much yes. I will go further and say that only boolean
variables should use the above syntax. Using
	if (i)
where 'i' is a non-boolean integer instead of
	if (0 != i)
makes me question what the programmer wanted. Since integers
do not have clear names for true/false logic (booleans usually
will be called something like 'have_brain" etc.) the simple
'if (i)' may just as well be a miswritten 'if (!i)' - and I
caught a few of these bugs in my time.

The whole point is not about "does the blind law of ANSI accepts
this" because ANSI is written to be as permissible as possible
so as not to invalidate deprecated bad style.

The whole point is about writing code that has a clear meaning, and
using different forms for a zero integer and a generic empty pointer
is a good way to force the writer to think and do the right thing.

Fact is that some code I can read as I see the snippet and without
having to resort to too much header checking etc, while other code
is a pain to go through (and don't even start me on consistent
indentation).

And just to be clear. ANSI says this is valid
	char 	*p;
	int	i;
	...
	i[p] = 1;
Do you encourage this too? See what I mean? OK, so we all read
the reference to the obfuscation competition...

> And do we then need to initialize static pointers to NULL instead
> of letting them be implicitly 0.

I surely insist on this. Implicit is just not good enough to show
that you gave it enough thought.

> Is doing memset(&(struct with_embeded_pointers), 0, sizeof(struct))
> also wrong?

You probably think that 'calloc' is enough for initializing an
object. I fixed enough problems from this. You declare a struct
- write a struct_clear function right away. It is worth it's
wordcount in gold.

> I don't see that 0 is WRONG.  I do agree that ``((void *)0)'' is
> slightly more typesafe than ``0'', but since we don't have a lot of
> (void *) pointers in the kernel that is still the WRONG pointer type.
> 
> I do see that NULL has superior readability and maintainability and so
> should be encouraged by Documentation/CodingStyle.
> 
> The B and K&R roots of a simple single type language are what give C
> most of it's simplicity flexibility and power.  Please don't be so
> eager to throw those out.

Yes, it was simple and flexible. It was a damn great improvement
over assembly. However, by now it is clear that stronger typing
is a good thing. Kernel code, where bugs have a high cost, is a good
place to apply stricter rules than usual.

> You want to be so typesafe it sounds like you want to recode the
> kernel in Pascal.  You've written sparse, so it should be just a little
> more work to write a Pascal backend.  After that the kernel will be so
> typesafe the compiler won't let us poor programmers get it wrong.

Did we forget the smiley?

--
Eyal Lebedinsky		(eyal@eyal.emu.id.au)

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Alexandre Oliva]

On Jul 10, 2004, Eyal Lebedinsky <eyal@eyal.emu.id.au> wrote:

> Very much yes. I will go further and say that only boolean
> variables should use the above syntax. Using
> 	if (i)
> where 'i' is a non-boolean integer instead of
> 	if (0 != i)
> makes me question what the programmer wanted. Since integers
> do not have clear names for true/false logic (booleans usually
> will be called something like 'have_brain" etc.) the simple
> 'if (i)' may just as well be a miswritten 'if (!i)' - and I
> caught a few of these bugs in my time.

So how about pushing for writing (i == 1) if i is boolean, to be
clearer?  It's often nice to be able to tell whether a boolean
variable is strict 0/1 or just zero/non-zero, when you're thinking of
switching to 3+-state logic.  But guess what, i == 1 is far less
efficient than i != 0 on many architectures.  It's an unfortunate
trade-off you have to make in this case.

-- 
Alexandre Oliva             http://www.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer   aoliva@{redhat.com, gcc.gnu.org}
Free Software Evangelist  oliva@{lsd.ic.unicamp.br, gnu.org}

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Paul Jackson]

(off-topic alert)

>	if (0 != i)

Does anyone know of the origins of writing such tests this way, rather
than:

	if (i != 0)

I read the first as testing whether "0" has a certain property, which is
a silly thing to test, since the properties of "0" are rather constant.

The second form I read as testing a property of "i" - much more
interesting.  Logically, the same, of course.  Just a question of which
form is more idiomatic.

Back in the days when it was Ken, Dennis and Brian, not K & R, I don't
recall seeing the first form used much.  Even now I see _zero_ matches
on "if (0 " in kernel or mm - only in arch, drivers, net, scripts, and
sound (with a single time.h exception).

If I were Linus, I'd vote the first form off the island.  Then again,
if I were Linus, you would never have heard of Linux ;).

-- 
                          I won't rest till it's the best ...
                          Programmer, Linux Scalability
                          Paul Jackson <pj@sgi.com> 1.650.933.1373

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Tim Wright]

It was because

if (0 = i)

will give an error where you obviously meant '=='. It prevents
accidental "assignment in conditional context".

Tim

On Sat, 2004-07-10 at 16:52, Paul Jackson wrote:
> (off-topic alert)
> 
> >	if (0 != i)
> 
> Does anyone know of the origins of writing such tests this way, rather
> than:
> 
> 	if (i != 0)
> 
> I read the first as testing whether "0" has a certain property, which is
> a silly thing to test, since the properties of "0" are rather constant.
> 
> The second form I read as testing a property of "i" - much more
> interesting.  Logically, the same, of course.  Just a question of which
> form is more idiomatic.
> 
> Back in the days when it was Ken, Dennis and Brian, not K & R, I don't
> recall seeing the first form used much.  Even now I see _zero_ matches
> on "if (0 " in kernel or mm - only in arch, drivers, net, scripts, and
> sound (with a single time.h exception).
> 
> If I were Linus, I'd vote the first form off the island.  Then again,
> if I were Linus, you would never have heard of Linux ;).

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Bill Davidsen]

Eric W. Biederman wrote:

> Is doing memset(&(struct with_embeded_pointers), 0, sizeof(struct))
> also wrong?
> 
> I don't see that 0 is WRONG.  I do agree that ``((void *)0)'' is
> slightly more typesafe than ``0'', but since we don't have a lot of
> (void *) pointers in the kernel that is still the WRONG pointer type.
> 
> I do see that NULL has superior readability and maintainability and so
> should be encouraged by Documentation/CodingStyle.
> 
> The B and K&R roots of a simple single type language are what give C
> most of it's simplicity flexibility and power.  Please don't be so
> eager to throw those out.  
> 
> You want to be so typesafe it sounds like you want to recode the
> kernel in Pascal.  You've written sparse, so it should be just a little
> more work to write a Pascal backend.  After that the kernel will be so
> typesafe the compiler won't let us poor programmers get it wrong.

You say that as if it were a bad thing...

I don't have a current C standard handy, but I believe there's a 
requirement that otherwise uninitialized static pointers be initialized 
to NULL even if that isn't "all bits off."

-- 
    -bill davidsen (davidsen@tmr.com)
"The secret to procrastination is to put things off until the
  last possible moment - but no longer"  -me

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Florian Weimer]

* Linus Torvalds:

> "0" is an _integer_. It's not a pointer.

It's neither.  It's a literal.  Context may turn it into something
else, though.

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Richard B. Johnson]

On Thu, 8 Jul 2004, Herbert Xu wrote:

> Chris Wright <chrisw@osdl.org> wrote:
> > Fixup another round of sparse warnings of the type:
> >        warning: Using plain integer as NULL pointer
>
> What's wrong with using 0 as the NULL pointer? In contexts where
> a plain 0 is unsafe, NULL is usually unsafe as well.
>
> Cheers,
> --

Because NULL is a valid pointer value. 0 is not. If you were
to make 0 valid, you would use "(void *)0", which is what
NULL just happens to be in all known architectures so far,
although that could change in an alternate universe.


Cheers,
Dick Johnson
Penguin : Linux version 2.4.26 on an i686 machine (5570.56 BogoMips).
            Note 96.31% of all statistics are fiction.

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Michael Buesch]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoting "Richard B. Johnson" <root@chaos.analogic.com>:
> Because NULL is a valid pointer value. 0 is not. If you were
> to make 0 valid, you would use "(void *)0", which is what
> NULL just happens to be in all known architectures so far,
> although that could change in an alternate universe.

No, that is not true.
In C/C++ this is true:
NULL == 0

You can _always_ use 0 instead of NULL. The use of NULL is
_only_ for readability reasons. When you assign 0 to a
pointer, the compiler knows that you want to assign a
NULL-pointer and not the value 0.
Even on architectures where the NULL-pointer is not
represented as 0 in memory (another bitmask), it's still
valid to assign 0 to a pointer, because the compiler
_knows_ that you are handling with a pointer and does
The Right Thing (tm).

- -- 
Regards Michael Buesch  [ http://www.tuxsoft.de.vu ]


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.4 (GNU/Linux)

iD8DBQFA7S/mFGK1OIvVOP4RAoJ4AKCVY9QbxaXEjGthqGXpoN1Wqw8bzACgwzW/
AZVhE8MYOGNwKC/k74qZUBw=
=Vy7x
-----END PGP SIGNATURE-----

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Martin Zwickel]

[-- Attachment #1: Type: text/plain, Size: 796 bytes --]

On Thu, 8 Jul 2004 13:28:38 +0200
Michael Buesch <mbuesch@freenet.de> bubbled:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Quoting "Richard B. Johnson" <root@chaos.analogic.com>:
> > Because NULL is a valid pointer value. 0 is not. If you were
> > to make 0 valid, you would use "(void *)0", which is what
> > NULL just happens to be in all known architectures so far,
> > although that could change in an alternate universe.
> 
> No, that is not true.
> In C/C++ this is true:
> NULL == 0


hmm...

include/linux/stddef.h:

#undef NULL
#if defined(__cplusplus)
#define NULL 0
#else
#define NULL ((void *)0)
#endif

-- 
MyExcuse:
fractal radiation jamming the backbone

Martin Zwickel <martin.zwickel@technotrend.de>
Research & Development

TechnoTrend AG <http://www.technotrend.de>

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Michael Buesch]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoting Martin Zwickel <martin.zwickel@technotrend.de>:
> include/linux/stddef.h:
>
> #undef NULL
> #if defined(__cplusplus)
> #define NULL 0
> #else
> #define NULL ((void *)0)
> #endif

Yes, I never understood the reason for this ugly
#if defined(__cplusplus) here.
It works, but is IMHO unneccessary.

- --
Regards Michael Buesch  [ http://www.tuxsoft.de.vu ]


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.4 (GNU/Linux)

iD8DBQFA7Ti+FGK1OIvVOP4RAgM/AJ9zsaNf0kKrQTq/a5R89pdjB8+/fgCfbS1p
1m6bM+MX3Dyg3lKcUK9qgRE=
=GxfG
-----END PGP SIGNATURE-----

[OT] NULL versus 0 (Re: [PATCH] Use NULL instead of integer 0 in security/selinux/)

[Kari Hurtta]

-- Start of PGP signed section.
> Quoting Martin Zwickel <martin.zwickel@technotrend.de>:
> > include/linux/stddef.h:
> >
> > #undef NULL
> > #if defined(__cplusplus)
> > #define NULL 0
> > #else
> > #define NULL ((void *)0)
> > #endif
> 
> Yes, I never understood the reason for this ugly
> #if defined(__cplusplus) here.
> It works, but is IMHO unneccessary.
> 

(This is is off topic, because kernel is not C++, but C).

Some quotations from  Bjarne Stroustrup: The C++ Programming Language
(Third Edition),

   p. 843:    Note that a pointer to function or a pointer to member
              cannot be implicity converted to a void *.

   p. 844:    A constant expression (§C.5) that evaluates to 0 can
              be implicitly converted to any pointer or pointer
              to member type (§5.1.1.).


   p. 88:     In C, it has been popular to define a macro NULL to
              represent the zero pointer. Because of C++'s tighter 
              type checking, the use of plain 0, rather than any
              suggested NULL macro, leads to fewer problems. If you 
              feel you must define NULL, use

                  const int NULL = 0;

(typos mine.)

/ Kari Hurtta

Re: [OT] NULL versus 0 (Re: [PATCH] Use NULL instead of integer 0 in security/selinux/)

[Michael Buesch]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

That's all OK, fine and correct, but
#define NULL 0
would work for both, C and C++ as far as I can see.
Am I missing some special case?


Quoting Kari Hurtta <hurtta+zz1@leija.mh.fmi.fi>:
> -- Start of PGP signed section.
> > Quoting Martin Zwickel <martin.zwickel@technotrend.de>:
> > > include/linux/stddef.h:
> > >
> > > #undef NULL
> > > #if defined(__cplusplus)
> > > #define NULL 0
> > > #else
> > > #define NULL ((void *)0)
> > > #endif
> >
> > Yes, I never understood the reason for this ugly
> > #if defined(__cplusplus) here.
> > It works, but is IMHO unneccessary.
> >
>
> (This is is off topic, because kernel is not C++, but C).
>
> Some quotations from  Bjarne Stroustrup: The C++ Programming Language
> (Third Edition),
>
>    p. 843:    Note that a pointer to function or a pointer to member
>               cannot be implicity converted to a void *.
>
>    p. 844:    A constant expression (§C.5) that evaluates to 0 can
>               be implicitly converted to any pointer or pointer
>               to member type (§5.1.1.).
>
>
>    p. 88:     In C, it has been popular to define a macro NULL to
>               represent the zero pointer. Because of C++'s tighter
>               type checking, the use of plain 0, rather than any
>               suggested NULL macro, leads to fewer problems. If you
>               feel you must define NULL, use
>
>                   const int NULL = 0;
>
> (typos mine.)
>
> / Kari Hurtta
>
>

- --
Regards Michael Buesch  [ http://www.tuxsoft.de.vu ]


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.4 (GNU/Linux)

iD8DBQFA7UEvFGK1OIvVOP4RAua2AKCCwyC3TzasTBCbPQLaKaU47UJEbACZAY4P
wd6n6AvSuJ+ThZE/Msbs9x0=
=jvQR
-----END PGP SIGNATURE-----

Re: [OT] NULL versus 0 (Re: [PATCH] Use NULL instead of integer 0 in security/selinux/)

[Kari Hurtta]

-- Start of PGP signed section.
> That's all OK, fine and correct, but
> #define NULL 0
> would work for both, C and C++ as far as I can see.
> Am I missing some special case?

As far I know it does not work on C when it is
used as  argument of function and function
have not prototype or function's prototype have ...

In that case compiler do not know that pointer
is required instead of integer. 

However this is just "as far I know", now I have not
in hand reference (or I did not found good quotation.)

/ Kari Hurtta

> 
> Quoting Kari Hurtta <hurtta+zz1@leija.mh.fmi.fi>:
> > -- Start of PGP signed section.
> > > Quoting Martin Zwickel <martin.zwickel@technotrend.de>:
> > > > include/linux/stddef.h:
> > > >
> > > > #undef NULL
> > > > #if defined(__cplusplus)
> > > > #define NULL 0
> > > > #else
> > > > #define NULL ((void *)0)
> > > > #endif
> > >
> > > Yes, I never understood the reason for this ugly
> > > #if defined(__cplusplus) here.
> > > It works, but is IMHO unneccessary.
> > >
> >
> > (This is is off topic, because kernel is not C++, but C).
> >
> > Some quotations from  Bjarne Stroustrup: The C++ Programming Language
> > (Third Edition),
> >
> >    p. 843:    Note that a pointer to function or a pointer to member
> >               cannot be implicity converted to a void *.
> >
> >    p. 844:    A constant expression (§C.5) that evaluates to 0 can
> >               be implicitly converted to any pointer or pointer
> >               to member type (§5.1.1.).
> >
> >
> >    p. 88:     In C, it has been popular to define a macro NULL to
> >               represent the zero pointer. Because of C++'s tighter
> >               type checking, the use of plain 0, rather than any
> >               suggested NULL macro, leads to fewer problems. If you
> >               feel you must define NULL, use
> >
> >                   const int NULL = 0;
> >
> > (typos mine.)
> >
> > / Kari Hurtta
> >
> >
> 
> --
> Regards Michael Buesch  [ http://www.tuxsoft.de.vu ]
> 
> 
-- End of PGP signed section, PGP failed!

Re: [OT] NULL versus 0 (Re: [PATCH] Use NULL instead of integer 0 in security/selinux/)

[Herbert Xu]

On Thu, Jul 08, 2004 at 03:57:48PM +0300, Kari Hurtta wrote:
> 
> As far I know it does not work on C when it is
> used as  argument of function and function
> have not prototype or function's prototype have ...

In that case NULL is wrong anyway since not all pointers are equivalent.
You need to cast 0 or NULL to the exact pointer type required by that
function.
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Florian Weimer]

* Michael Buesch:

> Yes, I never understood the reason for this ugly
> #if defined(__cplusplus) here.
> It works, but is IMHO unneccessary.

It's necessary because in C++, (void *)0 is not implicitly converted
to other pointer types.  Having to write static_cast<T*>(NULL) is
certainly a bit too verbose.

There's also a C++ DR about this topic because people feel that there
should be a 0/NULL distinction for overloading.  But this is
completely irrelevant to C code.

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[P. Benie]

On Thu, 8 Jul 2004, Richard B. Johnson wrote:
> On Thu, 8 Jul 2004, Herbert Xu wrote:
> > What's wrong with using 0 as the NULL pointer?
>
> Because NULL is a valid pointer value. 0 is not. If you were
> to make 0 valid, you would use "(void *)0", which is what
> NULL just happens to be in all known architectures so far,
> although that could change in an alternate universe.

False. "An integer constant expressions with the value 0, or such an
expression cast to type void *, is called a null pointer constant. If a
null pointer constant is assigned to or compared for equality with a
pointer, the constant is converted to a pointer of that type", and "Any
two null pointers shall compare equal."

In other words, when you use 0 as a null pointer, you really do get a null
pointer. If you are working on an architecture where the bit pattern of
the integer 0 and null pointers are not the same, the compiler will
perform the appropriate conversion for you, so it is always correct to
define NULL as (void *)0.

Personally, I always use 0 and NULL for integers and null pointers
respectively, but that's because of long estalished conventions that make
the code readabile, rather than anything to do with validity of the code.

Peter

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Richard B. Johnson]

On Thu, 8 Jul 2004, P. Benie wrote:

> On Thu, 8 Jul 2004, Richard B. Johnson wrote:
> > On Thu, 8 Jul 2004, Herbert Xu wrote:
> > > What's wrong with using 0 as the NULL pointer?
> >
> > Because NULL is a valid pointer value. 0 is not. If you were
> > to make 0 valid, you would use "(void *)0", which is what
> > NULL just happens to be in all known architectures so far,
> > although that could change in an alternate universe.
>
> False. "An integer constant expressions with the value 0, or such an
> expression cast to type void *, is called a null pointer constant. If a
> null pointer constant is assigned to or compared for equality with a
> pointer, the constant is converted to a pointer of that type", and "Any
> two null pointers shall compare equal."
>
> In other words, when you use 0 as a null pointer, you really do get a null
> pointer. If you are working on an architecture where the bit pattern of
> the integer 0 and null pointers are not the same, the compiler will
> perform the appropriate conversion for you, so it is always correct to
> define NULL as (void *)0.

That's NOT what is says. It states that a NULL pointer is converted to
the appropriate type before any comparison is made. It does NOT say
that 0 is a valid null-pointer.

>
> Personally, I always use 0 and NULL for integers and null pointers
> respectively, but that's because of long estalished conventions that make
> the code readabile, rather than anything to do with validity of the code.
>
> Peter
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

Cheers,
Dick Johnson
Penguin : Linux version 2.4.26 on an i686 machine (5570.56 BogoMips).
            Note 96.31% of all statistics are fiction.

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Michael Poole]

Richard B. Johnson writes:

> On Thu, 8 Jul 2004, P. Benie wrote:
>
>> False. "An integer constant expressions with the value 0, or such an
>> expression cast to type void *, is called a null pointer constant. If a
>> null pointer constant is assigned to or compared for equality with a
>> pointer, the constant is converted to a pointer of that type", and "Any
>> two null pointers shall compare equal."
>>
>> In other words, when you use 0 as a null pointer, you really do get a null
>> pointer. If you are working on an architecture where the bit pattern of
>> the integer 0 and null pointers are not the same, the compiler will
>> perform the appropriate conversion for you, so it is always correct to
>> define NULL as (void *)0.
>
> That's NOT what is says. It states that a NULL pointer is converted to
> the appropriate type before any comparison is made. It does NOT say
> that 0 is a valid null-pointer.

Could you please elaborate the rules of English in which "An integer
constant expresion with the value 0 [...] is called a null pointer
constant" does not mean that 0 is a null pointer?  0 is certainly an
integer constant expression with the value 0, so there must be
something extraordinarily subtle in the second half of the sentence.

Michael

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[P. Benie]

On Thu, 8 Jul 2004, Michael Poole wrote:
> Could you please elaborate the rules of English in which "An integer
> constant expresion with the value 0 [...] is called a null pointer
> constant" does not mean that 0 is a null pointer?  0 is certainly an
> integer constant expression with the value 0, so there must be
> something extraordinarily subtle in the second half of the sentence.

He's emphasising the difference between "null pointer constant"  and "null
pointer", however NULL is defined as "an implementation-defined null
pointer constant", so any subtle issues regarding 0 apply equally well to
NULL.

Someone pointed out that there can be a difference between 0 and NULL when
passing the value to a function lacking a prototype. There's no guarantee
that the null pointers of different types have the same representation, so
passing NULL into such a function without an explicit cast would also be
incorrect.

Peter

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Andreas Schwab]

Michael Poole <mdpoole@troilus.org> writes:

> Could you please elaborate the rules of English in which "An integer
> constant expresion with the value 0 [...] is called a null pointer
> constant" does not mean that 0 is a null pointer?

Null pointer != null pointer constant.  The latter is rather a syntactical
construct without a real value.  The process of converting a null pointer
constant to a null pointer is the point where the decision is made about
the final value and type.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Andrea Arcangeli]

On Thu, Jul 08, 2004 at 10:32:32AM -0400, Richard B. Johnson wrote:
> On Thu, 8 Jul 2004, P. Benie wrote:
> > the integer 0 and null pointers are not the same, the compiler will
> > perform the appropriate conversion for you, so it is always correct to
> > define NULL as (void *)0.
> 
> That's NOT what is says. It states that a NULL pointer is converted to
> the appropriate type before any comparison is made. It does NOT say
> that 0 is a valid null-pointer.

0 when casted or assigned to a pointer will be converted to the null
pointer value by the compiler, that's why doing !ptr is equivalent to
ptr == NULL, even if ptr points to address 0xffffffff virtual in
reality. Still NULL is set to (void *) 0, or alternatively (void *) -1UL
(thought the latter I'm not 100% sure but I think it'll work in such a
case).

It's mostly theory though, I've never seen an arch with a compiler with
a null pointer not actually meaning virtual address 0UL (that's why we
used a C-breaking #define NULL ((void*)-1UL) once we needed it).

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Andrea Arcangeli]

On Thu, Jul 08, 2004 at 12:43:16PM +0100, P. Benie wrote:
> the integer 0 and null pointers are not the same, the compiler will
> perform the appropriate conversion for you, so it is always correct to
> define NULL as (void *)0.

exactly, the compiler knows about that.

> Personally, I always use 0 and NULL for integers and null pointers
> respectively, but that's because of long estalished conventions that make
> the code readabile, rather than anything to do with validity of the code.

Yep.

Re: [PATCH] Use NULL instead of integer 0 in security/selinux/

[Andrea Arcangeli]

On Thu, Jul 08, 2004 at 07:10:12AM -0400, Richard B. Johnson wrote:
> Because NULL is a valid pointer value. 0 is not. If you were

ah you're doing my same mistake! ;) Just read the email I just posted on
this thread.