2007-08-02 11:32:42

by Miklos Szeredi

[permalink] [raw]
Subject: [PATCH] type safe allocator

From: Miklos Szeredi <[email protected]>

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 <[email protected]>
---

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


2007-08-02 12:04:18

by Alexey Dobriyan

[permalink] [raw]
Subject: Re: [PATCH] type safe allocator

On 8/2/07, Miklos Szeredi <[email protected]> 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.

2007-08-02 12:24:54

by Jan Engelhardt

[permalink] [raw]
Subject: Re: [PATCH] type safe allocator


On Aug 2 2007 16:04, Alexey Dobriyan wrote:
>On 8/2/07, Miklos Szeredi <[email protected]> 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
--

2007-08-02 13:07:31

by Miklos Szeredi

[permalink] [raw]
Subject: Re: [PATCH] type safe allocator

> On Aug 2 2007 16:04, Alexey Dobriyan wrote:
> >On 8/2/07, Miklos Szeredi <[email protected]> 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

2007-08-02 13:36:09

by Jan Engelhardt

[permalink] [raw]
Subject: Re: [PATCH] type safe allocator


On Aug 2 2007 15:06, Miklos Szeredi wrote:
>> On Aug 2 2007 16:04, Alexey Dobriyan wrote:
>> >On 8/2/07, Miklos Szeredi <[email protected]> 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
--

2007-08-02 13:48:16

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH] type safe allocator

On Thu, 2007-08-02 at 16:04 +0400, Alexey Dobriyan wrote:
> On 8/2/07, Miklos Szeredi <[email protected]> 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?


2007-08-02 13:50:05

by Miklos Szeredi

[permalink] [raw]
Subject: Re: [PATCH] type safe allocator

> >> On Aug 2 2007 16:04, Alexey Dobriyan wrote:
> >> >On 8/2/07, Miklos Szeredi <[email protected]> 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

2007-08-02 14:07:00

by Bernd Petrovitsch

[permalink] [raw]
Subject: Re: [PATCH] type safe allocator

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 <[email protected]> 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


2007-08-02 14:09:11

by Jan Engelhardt

[permalink] [raw]
Subject: Re: [PATCH] type safe allocator


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
--

2007-08-02 18:37:23

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] type safe allocator

On Thu, 02 Aug 2007 13:31:56 +0200
Miklos Szeredi <[email protected]> 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.

2007-08-02 18:48:51

by Miklos Szeredi

[permalink] [raw]
Subject: Re: [PATCH] type safe allocator

> > 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