[RFC PATCH] type safe allocator

[RFC PATCH] type safe allocator

[Miklos Szeredi]

I wonder why we don't have type safe object allocators a-la new() in
C++ or g_new() in glib?

  fooptr = k_new(struct foo, GFP_KERNEL);

is nicer and more descriptive than

  fooptr = kmalloc(sizeof(*fooptr), GFP_KERNEL);

and more safe than

  fooptr = kmalloc(sizeof(struct foo), GFP_KERNEL);

And we have zillions of both variants.

Note, I'm not advocating mass replacement, but using this in new code,
and gradually converting old ones whenever they need touching anyway.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---

Index: linux-2.6.22/include/linux/slab.h
===================================================================
--- linux-2.6.22.orig/include/linux/slab.h	2007-07-09 01:32:17.000000000 +0200
+++ linux-2.6.22/include/linux/slab.h	2007-08-01 10:42:45.000000000 +0200
@@ -110,6 +110,38 @@ static inline void *kcalloc(size_t n, si
 	return __kzalloc(n * size, flags);
 }
 
+/**
+ * k_new - allocate given type object
+ * @type: the type of the object to allocate
+ * @flags: the type of memory to allocate.
+ */
+#define k_new(type, flags) ((type *) kmalloc(sizeof(type), flags))
+
+/**
+ * k_new0 - allocate given type object, zero out allocated space
+ * @type: the type of the object to allocate
+ * @flags: the type of memory to allocate.
+ */
+#define k_new0(type, flags) ((type *) kzalloc(sizeof(type), flags))
+
+/**
+ * k_new_array - allocate array of given type object
+ * @type: the type of the object to allocate
+ * @len: the length of the array
+ * @flags: the type of memory to allocate.
+ */
+#define k_new_array(type, len, flags) \
+	((type *) kmalloc(sizeof(type) * (len), flags))
+
+/**
+ * k_new0_array - allocate array of given type object, zero out allocated space
+ * @type: the type of the object to allocate
+ * @len: the length of the array
+ * @flags: the type of memory to allocate.
+ */
+#define k_new0_array(type, len, flags) \
+	((type *) kzalloc(sizeof(type) * (len), flags))
+
 /*
  * Allocator specific definitions. These are mainly used to establish optimized
  * ways to convert kmalloc() calls to kmem_cache_alloc() invocations by selecting

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [RFC PATCH] type safe allocator

[Adrian Bunk]

On Wed, Aug 01, 2007 at 11:06:46AM +0200, Miklos Szeredi wrote:
> I wonder why we don't have type safe object allocators a-la new() in
> C++ or g_new() in glib?
> 
>   fooptr = k_new(struct foo, GFP_KERNEL);
> 
> is nicer and more descriptive than
> 
>   fooptr = kmalloc(sizeof(*fooptr), GFP_KERNEL);
>...

But it's much more likely to break when someone converts fooptr to a 
different struct.

It might not be a common case but it sometimes happens - and your type 
safe variant introduces the possibility for really nasty bugs.

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [RFC PATCH] type safe allocator

[Miklos Szeredi]

> > I wonder why we don't have type safe object allocators a-la new() in
> > C++ or g_new() in glib?
> > 
> >   fooptr = k_new(struct foo, GFP_KERNEL);
> > 
> > is nicer and more descriptive than
> > 
> >   fooptr = kmalloc(sizeof(*fooptr), GFP_KERNEL);
> >...
> 
> But it's much more likely to break when someone converts fooptr to a 
> different struct.
> 
> It might not be a common case but it sometimes happens - and your type 
> safe variant introduces the possibility for really nasty bugs.

The compiler would emit a warning about assigning to a pointer of
different type.  That's a fairly strong hint that something just
broke.

Miklos

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [RFC PATCH] type safe allocator

[Miklos Szeredi]

> > I wonder why we don't have type safe object allocators a-la new() in
> > C++ or g_new() in glib?
> > 
> >   fooptr = k_new(struct foo, GFP_KERNEL);
> > 
> > is nicer and more descriptive than
> > 
> >   fooptr = kmalloc(sizeof(*fooptr), GFP_KERNEL);
> > 
> > and more safe than
> > 
> >   fooptr = kmalloc(sizeof(struct foo), GFP_KERNEL);
> 
> How is it more safe? It seems 100% equivalent to me,
> just a different syntax.

Note the (type *) cast:

#define k_new(type, flags) ((type *) kmalloc(sizeof(type), flags))

Miklos

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [RFC PATCH] type safe allocator

[Andi Kleen]

Miklos Szeredi <miklos@szeredi.hu> writes:

> I wonder why we don't have type safe object allocators a-la new() in
> C++ or g_new() in glib?
> 
>   fooptr = k_new(struct foo, GFP_KERNEL);
> 
> is nicer and more descriptive than
> 
>   fooptr = kmalloc(sizeof(*fooptr), GFP_KERNEL);
> 
> and more safe than
> 
>   fooptr = kmalloc(sizeof(struct foo), GFP_KERNEL);

How is it more safe? It seems 100% equivalent to me,
just a different syntax.

> 
> And we have zillions of both variants.

In my own non kernel code i tend to define a pascal style NEW()

#define NEW(p) ((p) = malloc(sizeof(*(p))))

But I'm not sure such a untraditional solution would too popular.

Also I don't think we have too many bugs in this area anyways; so
it might be better to concentrate on more fruitful areas.

-Andi

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [RFC PATCH] type safe allocator

[Miklos Szeredi]

> > 
> > #define k_new(type, flags) ((type *) kmalloc(sizeof(type), flags))
> 
> The cast doesn't make it more safe in any way

I does, since a warning will be issued, if the type of the assigned
pointer doesn't match the requested allocation.

And yes, warnings are _very_ useful in C for enforcing type safety.

Miklos

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [RFC PATCH] type safe allocator

[Andi Kleen]

Miklos Szeredi <miklos@szeredi.hu> writes:
> 
> #define k_new(type, flags) ((type *) kmalloc(sizeof(type), flags))

The cast doesn't make it more safe in any way

(at least as long as you don't care about portability to C++;
the kernel doesn't)

-Andi

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [RFC PATCH] type safe allocator

[Jan Engelhardt]

On Aug 1 2007 12:45, Miklos Szeredi wrote:
>> > 
>> > #define k_new(type, flags) ((type *) kmalloc(sizeof(type), flags))
>> 
>> The cast doesn't make it more safe in any way
>
>I does, since a warning will be issued, if the type of the assigned
>pointer doesn't match the requested allocation.
>
>And yes, warnings are _very_ useful in C for enforcing type safety.

	void *p;
	p = (struct foo *)kmalloc(sizeof(struct foo), GFP_KERNEL);



	Jan
-- 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [RFC PATCH] type safe allocator

[Miklos Szeredi]

> >> > 
> >> > #define k_new(type, flags) ((type *) kmalloc(sizeof(type), flags))
> >> 
> >> The cast doesn't make it more safe in any way
> >
> >I does, since a warning will be issued, if the type of the assigned
> >pointer doesn't match the requested allocation.
> >
> >And yes, warnings are _very_ useful in C for enforcing type safety.
> 
> 	void *p;
> 	p = (struct foo *)kmalloc(sizeof(struct foo), GFP_KERNEL);

Using void pointers is _obviously_ not type safe.  What has that got
to do with k_new()?

Miklos

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [RFC PATCH] type safe allocator

[Linus Torvalds]

On Wed, 1 Aug 2007, Miklos Szeredi wrote:
>
> I wonder why we don't have type safe object allocators a-la new() in
> C++ or g_new() in glib?
> 
>   fooptr = k_new(struct foo, GFP_KERNEL);

I would object to this if only because of the horrible name.

C++ is not a good language to take ideas from, and "new()" was not it's 
best feature to begin with. "k_new()" is just disgusting.

I'd call it something like "alloc_struct()" instead, which tells you 
exactly what it's all about. Especially since we try to avoid typedefs in 
the kernel, and as a result, it's basically almost always a struct thing.

That said, I'm not at all sure it's worth it. Especially not with all the 
various variations on a theme (zeroed, arrays, etc etc).

Quite frankly, I suspect you would be better off just instrumenting 
"sparse" instead, and matching up the size of the allocation with the type 
it gets assigned to.

		Linus

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [RFC PATCH] type safe allocator

[Christoph Lameter]

On Wed, 1 Aug 2007, Miklos Szeredi wrote:

> I wonder why we don't have type safe object allocators a-la new() in
> C++ or g_new() in glib?
> 
>   fooptr = k_new(struct foo, GFP_KERNEL);
> 
> is nicer and more descriptive than
> 
>   fooptr = kmalloc(sizeof(*fooptr), GFP_KERNEL);
> 
> and more safe than
> 
>   fooptr = kmalloc(sizeof(struct foo), GFP_KERNEL);
> 
> And we have zillions of both variants.

Hmmm yes I think that would be good. However, please clean up the naming.
The variant on zeroing on zering get to be too much.

> + * k_new - allocate given type object
> + * @type: the type of the object to allocate
> + * @flags: the type of memory to allocate.
> + */
> +#define k_new(type, flags) ((type *) kmalloc(sizeof(type), flags))

kalloc?

> +
> + * k_new0 - allocate given type object, zero out allocated space
> + * @type: the type of the object to allocate
> + * @flags: the type of memory to allocate.
> + */
> +#define k_new0(type, flags) ((type *) kzalloc(sizeof(type), flags))

A new notation for zeroing! This is equivalent to

kalloc(type, flags | __GFP_ZERO)

maybe define new GFP_xxx instead?

> +/**
> + * k_new_array - allocate array of given type object
> + * @type: the type of the object to allocate
> + * @len: the length of the array
> + * @flags: the type of memory to allocate.
> + */
> +#define k_new_array(type, len, flags) \
> +	((type *) kmalloc(sizeof(type) * (len), flags))

We already have array initializations using kcalloc.

> +#define k_new0_array(type, len, flags) \
> +	((type *) kzalloc(sizeof(type) * (len), flags))

Same as before.


I do not see any _node variants?

How about the following minimal set


kmalloc(size, flags)		kalloc(struct, flags)
kmalloc_node(size, flags, node)	kalloc_node(struct, flags, node)


The array variants translate into kmalloc anyways and are used
in an inconsistent manner. Sometime this way sometimes the other. Leave 
them?

	kcalloc(n, size, flags) == kmalloc(size, flags)

Then kzalloc is equivalent to adding the __GFP_ZERO flag. Thus

	kzalloc(size, flags) == kmalloc(size, flags | __GFPZERO)

If you define a new flag like GFP_ZERO_ATOMIC and GFP_ZERO_KERNEL you 
could do

	kalloc(struct, GFP_ZERO_KERNEL)

instead of adding new variants?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [RFC PATCH] type safe allocator

[Miklos Szeredi]

> >
> > I wonder why we don't have type safe object allocators a-la new() in
> > C++ or g_new() in glib?
> > 
> >   fooptr = k_new(struct foo, GFP_KERNEL);
> 
> I would object to this if only because of the horrible name.
> 
> C++ is not a good language to take ideas from, and "new()" was not it's 
> best feature to begin with. "k_new()" is just disgusting.
> 
> I'd call it something like "alloc_struct()" instead, which tells you 
> exactly what it's all about. Especially since we try to avoid typedefs in 
> the kernel, and as a result, it's basically almost always a struct thing.

Yeah, I'm not strongly attached to the "new" name, although I got used
to it in glib.  The glib API is broken in lots of ways, but g_new()
and friends are nice and useful.

> That said, I'm not at all sure it's worth it. Especially not with all the 
> various variations on a theme (zeroed, arrays, etc etc).

The number of variations can be reduced to just zeroing/nonzeroing, by
making the array length mandatory.  That's what glib does in g_new().

> Quite frankly, I suspect you would be better off just instrumenting 
> "sparse" instead, and matching up the size of the allocation with the type 
> it gets assigned to.

But that just can't be done, because kmalloc() doesn't tell us the
_intent_ of the allocation.  The allocation could be for an array, or
for a struct with a variable length string at the end, or it could be
multiple structs concatenated.  We have all sorts of weird stuff in
there that sparse would not be able to handle.

That's why alloc_struct() is better: it describes the intention exacly
"allocate the given object and return an appropriately typed pointer".

While kmalloc() just says "allocate a given sized memory chunk and
return an untyped pointer".

Miklos

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [RFC PATCH] type safe allocator

[Satyam Sharma]

Hi Miklos,


On Wed, 1 Aug 2007, Miklos Szeredi wrote:

> I wonder why we don't have type safe object allocators a-la new() in
> C++ or g_new() in glib?
> 
>   fooptr = k_new(struct foo, GFP_KERNEL);
> 
> is nicer and more descriptive than
> 
>   fooptr = kmalloc(sizeof(*fooptr), GFP_KERNEL);
> 
> and more safe than
> 
>   fooptr = kmalloc(sizeof(struct foo), GFP_KERNEL);
> 
> And we have zillions of both variants.
> 
> Note, I'm not advocating mass replacement, but using this in new code,
> and gradually converting old ones whenever they need touching anyway.
> [...]
>  
> +/**
> + * k_new - allocate given type object
> + * @type: the type of the object to allocate
> + * @flags: the type of memory to allocate.
> + */
> +#define k_new(type, flags) ((type *) kmalloc(sizeof(type), flags))

What others already said, plus:

kmalloc()'ing sizeof(struct foo) is not always what we want in C either.

Several kernel structs have zero-length / variable-length array members
and space must be allocated for them only at alloc() time ... would be
impossible to make them work with this scheme.


Satyam

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [RFC PATCH] type safe allocator

[Miklos Szeredi]

> > I wonder why we don't have type safe object allocators a-la new() in
> > C++ or g_new() in glib?
> > 
> >   fooptr = k_new(struct foo, GFP_KERNEL);
> > 
> > is nicer and more descriptive than
> > 
> >   fooptr = kmalloc(sizeof(*fooptr), GFP_KERNEL);
> > 
> > and more safe than
> > 
> >   fooptr = kmalloc(sizeof(struct foo), GFP_KERNEL);
> > 
> > And we have zillions of both variants.
> 
> Hmmm yes I think that would be good. However, please clean up the naming.
> The variant on zeroing on zering get to be too much.

OK, there seems to be a consensus on that ;)

[snip]

> I do not see any _node variants?

Well, those are _very_ rare, I'd only add those if there's a demand
for them.

> The array variants translate into kmalloc anyways and are used
> in an inconsistent manner. Sometime this way sometimes the other. Leave 
> them?

If the too many variants are bothersome, then I'd rather just have the
array variant, and give 1 as an array size for the non-array case.

> 	kcalloc(n, size, flags) == kmalloc(size, flags)
> 
> Then kzalloc is equivalent to adding the __GFP_ZERO flag. Thus
> 
> 	kzalloc(size, flags) == kmalloc(size, flags | __GFPZERO)
> 
> If you define a new flag like GFP_ZERO_ATOMIC and GFP_ZERO_KERNEL you 
> could do
> 
> 	kalloc(struct, GFP_ZERO_KERNEL)
> 
> instead of adding new variants?

I don't really like this, introducing new gfp flags just makes
grepping harder.

I do think that at least having a zeroing and a non-zeroing variant
makes sense.

Miklos

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [RFC PATCH] type safe allocator

[Miklos Szeredi]

> >  
> > +/**
> > + * k_new - allocate given type object
> > + * @type: the type of the object to allocate
> > + * @flags: the type of memory to allocate.
> > + */
> > +#define k_new(type, flags) ((type *) kmalloc(sizeof(type), flags))
> 
> What others already said, plus:
> 
> kmalloc()'ing sizeof(struct foo) is not always what we want in C either.
> 
> Several kernel structs have zero-length / variable-length array members
> and space must be allocated for them only at alloc() time ... would be
> impossible to make them work with this scheme.

Exactly.  We can, and should use kmalloc() for that.

Miklos

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH] type safe allocator

[Miklos Szeredi, Miklos Szeredi]

The linux kernel doesn't have a type safe object allocator a-la new()
in C++ or g_new() in glib.

Introduce two helpers for this purpose:

   alloc_struct(type, gfp_flags);

   zalloc_struct(type, gfp_flags);

These macros take a type name (usually a 'struct foo') as first
argument and the usual gfp-flags as second argument.  They return a
pointer cast to 'type *'.

The traditional forms of allocating a structure are:

  fooptr = kmalloc(sizeof(*fooptr), ...);

  fooptr = kmalloc(sizeof(struct foo), ...);

The new form is preferred over these, because of it's type safety and
more descriptive nature.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---

Index: linux-2.6.22/include/linux/slab.h
===================================================================
--- linux-2.6.22.orig/include/linux/slab.h	2007-08-01 16:47:41.000000000 +0200
+++ linux-2.6.22/include/linux/slab.h	2007-08-02 12:55:20.000000000 +0200
@@ -110,6 +110,20 @@ static inline void *kcalloc(size_t n, si
 	return __kzalloc(n * size, flags);
 }
 
+/**
+ * alloc_struct - allocate given type object
+ * @type: the type of the object to allocate
+ * @flags: the type of memory to allocate.
+ */
+#define alloc_struct(type, flags) ((type *) kmalloc(sizeof(type), flags))
+
+/**
+ * zalloc_struct - allocate given type object, zero out the contents
+ * @type: the type of the object to allocate
+ * @flags: the type of memory to allocate.
+ */
+#define zalloc_struct(type, flags) ((type *) kzalloc(sizeof(type), flags))
+
 /*
  * Allocator specific definitions. These are mainly used to establish optimized
  * ways to convert kmalloc() calls to kmem_cache_alloc() invocations by selecting
Index: linux-2.6.22/Documentation/CodingStyle
===================================================================
--- linux-2.6.22.orig/Documentation/CodingStyle	2007-07-09 01:32:17.000000000 +0200
+++ linux-2.6.22/Documentation/CodingStyle	2007-08-02 13:03:48.000000000 +0200
@@ -631,21 +631,20 @@ Printing numbers in parentheses (%d) add
 		Chapter 14: Allocating memory
 
 The kernel provides the following general purpose memory allocators:
-kmalloc(), kzalloc(), kcalloc(), and vmalloc().  Please refer to the API
+kmalloc(), kzalloc(), kcalloc(), and vmalloc(), and the following
+helpers: alloc_struct() and zalloc_struct().  Please refer to the API
 documentation for further information about them.
 
-The preferred form for passing a size of a struct is the following:
+The preferred form for allocating a structure is the following:
 
-	p = kmalloc(sizeof(*p), ...);
+	p = alloc_struct(struct name, ...);
 
-The alternative form where struct name is spelled out hurts readability and
-introduces an opportunity for a bug when the pointer variable type is changed
-but the corresponding sizeof that is passed to a memory allocator is not.
-
-Casting the return value which is a void pointer is redundant. The conversion
-from void pointer to any other pointer type is guaranteed by the C programming
-language.
+The alternatives are less readable or introduce an opportunity for a bug
+when the pointer variable type is changed but the corresponding sizeof that
+is passed to a memory allocator is not.
 
+The return value of alloc_struct() and zalloc_struct() have the right type,
+so the compiler will warn if it is assigned to a pointer of different type.
 
 		Chapter 15: The inline disease
 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] type safe allocator

[Alexey Dobriyan]

On 8/2/07, Miklos Szeredi <miklos@szeredi.hu> wrote:
> The linux kernel doesn't have a type safe object allocator a-la new()
> in C++ or g_new() in glib.
>
> Introduce two helpers for this purpose:
>
>    alloc_struct(type, gfp_flags);
>
>    zalloc_struct(type, gfp_flags);

ick.

> These macros take a type name (usually a 'struct foo') as first
> argument

So one has to type struct twice.

> and the usual gfp-flags as second argument.  They return a
> pointer cast to 'type *'.
>
> The traditional forms of allocating a structure are:
>
>   fooptr = kmalloc(sizeof(*fooptr), ...);
>
>   fooptr = kmalloc(sizeof(struct foo), ...);

Key word is "traditional". Good traditional form which even half-competent
C programmers immediately parse in retina.

> The new form is preferred over these, because of it's type safety and
> more descriptive nature.

> +/**
> + * alloc_struct - allocate given type object
> + * @type: the type of the object to allocate
> + * @flags: the type of memory to allocate.
> + */
> +#define alloc_struct(type, flags) ((type *) kmalloc(sizeof(type), flags))

someone will write alloc_struct(int, GFP_KERNEL), I promise.

Can you play instead with something Lisp based which has infinetely more
potential for idioms.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [RFC PATCH] type safe allocator

[Al Viro]

On Thu, Aug 02, 2007 at 09:27:57AM +0200, Miklos Szeredi wrote:
> > Quite frankly, I suspect you would be better off just instrumenting 
> > "sparse" instead, and matching up the size of the allocation with the type 
> > it gets assigned to.
> 
> But that just can't be done, because kmalloc() doesn't tell us the
> _intent_ of the allocation.

Of course it can be done.  When argument has form sizeof(...), attach
the information about target of pointer to the resulting void *; have
? : between pointer to object and that one warn if types do not match,
? : between void * and that one lose that information, ? : between
two such warn when types do not match.  On assigment-type operations
(assignment, passing argument to function, initializer, return) when
the type of target is pointer to object (and not void *) warn if
types do not match.  Have typeof lose that information.

Not even hard to implement; just let us finish cleaning the lazy examination
logics up first (~5-6 patches away).

FWIW, I object against the original proposal, no matter how you name it.
Reason: we are introducing more magical constructs that have to be known
to human reader in order to parse the damn code.

Folks, this is serious.  _We_ might be used to having in effect a C dialect
with extensions implemented by preprocessor.  That's fine, but for a fresh
reader it becomes a problem; sure, they can dig in include/linux/*.h and
to some extent they clearly have to.  However, it doesn't come for free
and we really ought to keep that in mind - amount of local idioms (and
anything that doesn't look like a normal function call with normal arguments
_does_ become an idiom to be learnt before one can fluently RTFS) is a thing
to watch out for.

IOW, whenever we add to that pile we ought to look hard at whether it's worth
the trouble.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] type safe allocator

[Jan Engelhardt]

On Aug 2 2007 16:04, Alexey Dobriyan wrote:
>On 8/2/07, Miklos Szeredi <miklos@szeredi.hu> wrote:
>>   fooptr = kmalloc(sizeof(struct foo), ...);
>
>Key word is "traditional". Good traditional form which even half-competent
>C programmers immediately parse in retina.

And being aware of the potential type-unsafety makes programmers more
careful IMHO.

>
>> +/**
>> + * alloc_struct - allocate given type object
>> + * @type: the type of the object to allocate
>> + * @flags: the type of memory to allocate.
>> + */
>> +#define alloc_struct(type, flags) ((type *) kmalloc(sizeof(type), flags))

>someone will write alloc_struct(int, GFP_KERNEL), I promise.

and someone else will write

	struct complexthing foo;
	alloc_struct(foo, GFP_KERNEL);



	Jan
-- 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [RFC PATCH] type safe allocator

[Miklos Szeredi]

> Folks, this is serious.  _We_ might be used to having in effect a C dialect
> with extensions implemented by preprocessor.  That's fine, but for a fresh
> reader it becomes a problem; sure, they can dig in include/linux/*.h and
> to some extent they clearly have to.  However, it doesn't come for free
> and we really ought to keep that in mind - amount of local idioms (and
> anything that doesn't look like a normal function call with normal arguments
> _does_ become an idiom to be learnt before one can fluently RTFS) is a thing
> to watch out for.

That's why the g_new() form that glib uses makes some sense.  It
borrows an idiom from C++, and although we all know C++ is a horrid
language, to some extent lots of people are familiar with it.

> IOW, whenever we add to that pile we ought to look hard at whether it's worth
> the trouble.

Well, this is not some earth-shattering stuff, but I think it would be
good to have.  I got used to it in glib, and I miss it in linux.

I understand the knee-jerk reaction of most people who are unfamiliar
with it, and I can do nothing about that.  If there's no positive
feedback I'll just give up, it's not that I can't live with the
current situation.


Miklos

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] type safe allocator

[Miklos Szeredi]

> On Aug 2 2007 16:04, Alexey Dobriyan wrote:
> >On 8/2/07, Miklos Szeredi <miklos@szeredi.hu> wrote:
> >>   fooptr = kmalloc(sizeof(struct foo), ...);
> >
> >Key word is "traditional". Good traditional form which even half-competent
> >C programmers immediately parse in retina.
> 
> And being aware of the potential type-unsafety makes programmers more
> careful IMHO.

That's a _really_ good reason ;)

> >> +/**
> >> + * alloc_struct - allocate given type object
> >> + * @type: the type of the object to allocate
> >> + * @flags: the type of memory to allocate.
> >> + */
> >> +#define alloc_struct(type, flags) ((type *) kmalloc(sizeof(type), flags))
> 
> >someone will write alloc_struct(int, GFP_KERNEL), I promise.
> 
> and someone else will write
> 
> 	struct complexthing foo;
> 	alloc_struct(foo, GFP_KERNEL);

And the compiler will complain like mad about that.

Miklos

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] type safe allocator

[Jan Engelhardt]

On Aug 2 2007 15:06, Miklos Szeredi wrote:
>> On Aug 2 2007 16:04, Alexey Dobriyan wrote:
>> >On 8/2/07, Miklos Szeredi <miklos@szeredi.hu> wrote:
>> >>   fooptr = kmalloc(sizeof(struct foo), ...);
>> >
>> >Key word is "traditional". Good traditional form which even half-competent
>> >C programmers immediately parse in retina.
>> 
>> And being aware of the potential type-unsafety makes programmers more
>> careful IMHO.
>
>That's a _really_ good reason ;)

Yes, a good reason not to use g_new(), so people do get bitten when
they are doingitwrong.



	Jan
-- 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] type safe allocator

[Peter Zijlstra]

On Thu, 2007-08-02 at 16:04 +0400, Alexey Dobriyan wrote:
> On 8/2/07, Miklos Szeredi <miklos@szeredi.hu> wrote:
> > The linux kernel doesn't have a type safe object allocator a-la new()
> > in C++ or g_new() in glib.
> >
> > Introduce two helpers for this purpose:
> >
> >    alloc_struct(type, gfp_flags);
> >
> >    zalloc_struct(type, gfp_flags);
> 
> ick.
> 
> > These macros take a type name (usually a 'struct foo') as first
> > argument
> 
> So one has to type struct twice.

thrice in some cases like alloc_struct(struct task_struct, GFP_KERNEL)

I've always found this _struct postfix a little daft, perhaps its time
to let the janitors clean that out?


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] type safe allocator

[Miklos Szeredi]

> >> On Aug 2 2007 16:04, Alexey Dobriyan wrote:
> >> >On 8/2/07, Miklos Szeredi <miklos@szeredi.hu> wrote:
> >> >>   fooptr = kmalloc(sizeof(struct foo), ...);
> >> >
> >> >Key word is "traditional". Good traditional form which even half-competent
> >> >C programmers immediately parse in retina.
> >> 
> >> And being aware of the potential type-unsafety makes programmers more
> >> careful IMHO.
> >
> >That's a _really_ good reason ;)
> 
> Yes, a good reason not to use g_new(), so people do get bitten when
> they are doingitwrong.

Should we turn off all warnings then, to make people more careful
after constantly being bitten by stupid mistakes?

That's one way to think of it, yes.  But I think most would agree,
that we have better things to do than being careful about things that
the compiler can check for us.

Miklos

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] type safe allocator

[Bernd Petrovitsch]

On Thu, 2007-08-02 at 15:47 +0200, Peter Zijlstra wrote:
> On Thu, 2007-08-02 at 16:04 +0400, Alexey Dobriyan wrote:
> > On 8/2/07, Miklos Szeredi <miklos@szeredi.hu> wrote:
> > > The linux kernel doesn't have a type safe object allocator a-la new()
> > > in C++ or g_new() in glib.
> > >
> > > Introduce two helpers for this purpose:
> > >
> > >    alloc_struct(type, gfp_flags);
> > >
> > >    zalloc_struct(type, gfp_flags);
> > 
> > ick.
> > 
> > > These macros take a type name (usually a 'struct foo') as first
> > > argument
> > 
> > So one has to type struct twice.
> 
> thrice in some cases like alloc_struct(struct task_struct, GFP_KERNEL)

Save the explicit "struct" and put it into the macro (and force people
to not use typedefs).

#define alloc_struct(type, flags) ((type *)kmalloc(sizeof(struct type), (flags)))

Obious drawback: We may need alloc_union().

SCNR ...
	Bernd
-- 
Firmix Software GmbH                   http://www.firmix.at/
mobil: +43 664 4416156                 fax: +43 1 7890849-55
          Embedded Linux Development and Services


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] type safe allocator

[Jan Engelhardt]

On Aug 2 2007 16:06, Bernd Petrovitsch wrote:
>> thrice in some cases like alloc_struct(struct task_struct, GFP_KERNEL)
>
>Save the explicit "struct" and put it into the macro (and force people
>to not use typedefs).
>
>#define alloc_struct(type, flags) ((type *)kmalloc(sizeof(struct type), (flags)))

#define alloc_struct(type, flags) ((struct type *)kmalloc(sizeof(struct type), (flags)))

>Obious drawback: We may need alloc_union().
>SCNR.

And we still don't have something to allocate a bunch of ints.
[kmalloc(sizeof(int)*5,GFP)]


	Jan
-- 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [RFC PATCH] type safe allocator

[Linus Torvalds]

On Thu, 2 Aug 2007, Miklos Szeredi wrote:
> 
> The number of variations can be reduced to just zeroing/nonzeroing, by
> making the array length mandatory.  That's what glib does in g_new().

Quite frankly, you don't need the zeroing. That's what __GFP_ZERO does in 
the flags.

That said, I'm not at all sure that it's at all more readable to add some 
new abstraction layer and do

	struct random_struct *ptr;

	ptr = alloc_struct(random_struct, 1, GFP_KERNEL | __GFP_ZERO);

than just doing a

	ptr = kmalloc(sizeof(*ptr), GFP_KERNEL | __GFP_ZERO);

or

	ptr = kzalloc(sizeof(*ptr), GFP_KERNEL);

(and adding the zeroing variant of alloc_struct() just adds *more* 
confusing issues).

The fact is, type safety in this area is probably less important than the 
code just being readable. And have fifteen different interfaces to memory 
allocation just isn't ever going to readable - regardless of how good they 
are individually.

		Linus

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] type safe allocator

[Andrew Morton]

On Thu, 02 Aug 2007 13:31:56 +0200
Miklos Szeredi <miklos@szeredi.hu> wrote:

> The linux kernel doesn't have a type safe object allocator a-la new()
> in C++ or g_new() in glib.
> 
> Introduce two helpers for this purpose:
> 
>    alloc_struct(type, gfp_flags);
> 
>    zalloc_struct(type, gfp_flags);

whimper.

On a practical note, I'm still buried in convert-to-kzalloc patches, and
your proposal invites a two-year stream of 10,000 convert-to-alloc_struct
patches.

So if this goes in (and I can't say I'm terribly excited about the idea)
then I think we'd also need a maintainer who is going to handle all the
subsequent patches, run a git tree, (a quilt tree would be better, or maybe
a git tree with 100 branches), work with all the affected maintainers, make
sure there aren't clashes with other people's work and all that blah.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] type safe allocator

[Miklos Szeredi]

> > The linux kernel doesn't have a type safe object allocator a-la new()
> > in C++ or g_new() in glib.
> > 
> > Introduce two helpers for this purpose:
> > 
> >    alloc_struct(type, gfp_flags);
> > 
> >    zalloc_struct(type, gfp_flags);
> 
> whimper.
> 
> On a practical note, I'm still buried in convert-to-kzalloc patches, and
> your proposal invites a two-year stream of 10,000 convert-to-alloc_struct
> patches.
> 
> So if this goes in

No, I gave up.  It seems nobody likes the idea except me :(

Miklos

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [RFC PATCH] type safe allocator

[Christoph Lameter]

On Thu, 2 Aug 2007, Miklos Szeredi wrote:

> > If you define a new flag like GFP_ZERO_ATOMIC and GFP_ZERO_KERNEL you 
> > could do
> > 
> > 	kalloc(struct, GFP_ZERO_KERNEL)
> > 
> > instead of adding new variants?
> 
> I don't really like this, introducing new gfp flags just makes
> grepping harder.

The __GFP_ZERO flag has been around for a long time. GFP_ZERO_ATOMIC and 
GFP_ZERO_KERNEL or so could just be a shorthand notation.

Maybe

#define GFP_ZATOMIC (GFP_ATOMIC | __GFP_ZERO)
#define GFP_ZKERNEL (GFP_KERNEL | __GFP_ZERO)

?

> I do think that at least having a zeroing and a non-zeroing variant
> makes sense.

They require a duplication of the API and have led to inconsistencies 
because the complete API was not available with zeroing capabilities 
(there is still no kzalloc_node f.e.). 
Using a gfp flag allows all allocation functions to optionally zero data 
without having to define multiple functions.

The definition of new variants is a bit complicated since the allocator 
functions contain lots of smarts to do inline constant folding. This is 
necessary to determine the correct slab at compile time. I'd rather have 
as few of those as possible.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>