Hi,
is there any reason why kfree() takes a const pointer just to degrade it
with the call to slab_free()/__cache_free() again? The promise that the
pointee is not modified is just bogus in this case, anyway, isn't it?
Hannes
On Wed, 16 Jan 2008, Johannes Weiner wrote:
> is there any reason why kfree() takes a const pointer just to degrade it
> with the call to slab_free()/__cache_free() again? The promise that the
> pointee is not modified is just bogus in this case, anyway, isn't it?
The object is modified in various cases f.e. because of poisoning or the
need to store the free pointer. So its bogus, yes. Pekka?
On Mit, 2008-01-16 at 08:48 -0800, Christoph Lameter wrote:
> On Wed, 16 Jan 2008, Johannes Weiner wrote:
>
> > is there any reason why kfree() takes a const pointer just to degrade it
> > with the call to slab_free()/__cache_free() again? The promise that the
> > pointee is not modified is just bogus in this case, anyway, isn't it?
>
> The object is modified in various cases f.e. because of poisoning or the
> need to store the free pointer. So its bogus, yes. Pekka?
Technically one should be able to pass a "const $type *" (which may
have been a "non-const $type *" before but at some point in time it
became "const $type *") to kfree().
The (formerly) constant contents as such vanishes IMHO (and it is not
really "modified").
Poisoning and free memory handling is IMHO internal stuff to the free
memory management subsystem and basically unrelated to the "life" of the
pointered contents before it's death with kfree().
Bernd
--
Firmix Software GmbH http://www.firmix.at/
mobil: +43 664 4416156 fax: +43 1 7890849-55
Embedded Linux Development and Services
Hi,
On Wed, 16 Jan 2008, Johannes Weiner wrote:
> > is there any reason why kfree() takes a const pointer just to degrade it
> > with the call to slab_free()/__cache_free() again? The promise that the
> > pointee is not modified is just bogus in this case, anyway, isn't it?
On Wed, 16 Jan 2008, Christoph Lameter wrote:
> The object is modified in various cases f.e. because of poisoning or the
> need to store the free pointer. So its bogus, yes. Pekka?
Yeah, bogus, and has been that way for a long time according to git. I'm
ok with removing that (which would make it consistent with the user-space
equivalent free(3) function btw).
Pekka
On Wed, 16 Jan 2008, Johannes Weiner wrote:
>
> is there any reason why kfree() takes a const pointer just to degrade it
> with the call to slab_free()/__cache_free() again? The promise that the
> pointee is not modified is just bogus in this case, anyway, isn't it?
"const" has *never* been about the thing not being modified. Forget all
that claptrap. C does not have such a notion.
"const" is a pointer type issue, and is meant to make certain mis-uses
more visible at compile time. It has *no* other meaning, and anybody who
thinks it has is just setting himself up for problems.
In the particular case of kfree(), the pointer argument *should* be const,
and the fact that the C library gets this wrong is not the kernels
problem, it's a problem with the C library.
Why?
- From a very obvious and very *real* caller perspective, "free()" really
doesn't change the thing the pointer points to. It does something
totally different: it makes the *pointer* itself invalid.
In other words, if you think that "kfree()" changed the thing you
free'd, you're simply wrong. It did no such thing. The memory is 100%
the same, it's just that you cannot access it any more, and if you try,
you'll get somebody elses memory.
In other words, "kfree()" can be const.
- Anything that *can* take a const pointer should always do so.
Why? Because we want the types to be as tight as possible, and normal
code should need as few casts as possible.
Here's a question for you: let's say that you have a structure that
has a member that is never changed. To make that obvious, and to allow
the compiler to warn about mis-use of a pointer, the structure should
look something like
struct mystruct {
const char *name;
..
and let's look at what happens if the allocation of that const thing is
dynamic.
The *correct* way to do that is:
char *name = kmalloc(...)
/* Fill it in */
snprintf(name, ...)
mystruct->name = name;
and there are no casts anywhere, and you get exactly the semantics you
want: "name" itself isn't constant (it's obviously modified), but at
the same time the type system makes it very clear that trying to change
it through that mystruct member pointer is wrong.
How do you free it?
That's right, you do:
kfree(mystruct->name);
and this is why "kfree()" should take a const pointer. If it doesn't,
you have to add an *incorrect* and totally useless cast to code that
was correct.
So never believe that "const" is some guarantee that the memory under the
pointer doesn't change. That is *never* true. It has never been true in
C, since there can be arbitrary pointer aliases to that memory that aren't
actually const. If you think "const *p" means that the memory behind "p"
is immutable, you're simply wrong.
Anybody who thinks that kfree() cannot (or should not) be const doesn't
understand the C type system.
Linus
Hi,
Linus Torvalds <[email protected]> writes:
[...]
> "const" is a pointer type issue, and is meant to make certain mis-uses
> more visible at compile time. It has *no* other meaning, and anybody who
> thinks it has is just setting himself up for problems.
[...]
> - From a very obvious and very *real* caller perspective, "free()" really
> doesn't change the thing the pointer points to. It does something
> totally different: it makes the *pointer* itself invalid.
>
> In other words, if you think that "kfree()" changed the thing you
> free'd, you're simply wrong. It did no such thing. The memory is 100%
> the same, it's just that you cannot access it any more, and if you try,
> you'll get somebody elses memory.
>
> In other words, "kfree()" can be const.
[...]
> So never believe that "const" is some guarantee that the memory under the
> pointer doesn't change. That is *never* true. It has never been true in
> C, since there can be arbitrary pointer aliases to that memory that aren't
> actually const. If you think "const *p" means that the memory behind "p"
> is immutable, you're simply wrong.
Okay, I understood that now. A const qualifier just forbids modifying
the underlying memory _through this particular pointer_, right?
In the case of slub's kfree(), which takes a const pointer, you pass it
on to slab_free() which actually _DOES_ modification of the underlying
memory when linking the object to the freelist (as far as I understood
the code).
So if I got it right and you actually modify the memory you only got a
const pointer to, you reach a level where you _have to_ break this
policy and cast to a non-const pointer, as it is currently done in
kfree(). No?
Hannes
On Wed, 16 Jan 2008, Johannes Weiner wrote:
> So if I got it right and you actually modify the memory you only got a
> const pointer to, you reach a level where you _have to_ break this
> policy and cast to a non-const pointer, as it is currently done in
> kfree(). No?
Correct and we have gcc 4.2 currently spitting out warnings because of
casting to non const. Any idea how to convince gcc that this is okay?
On Wed, Jan 16, 2008 at 10:39:00AM -0800, Linus Torvalds wrote:
>
>
> On Wed, 16 Jan 2008, Johannes Weiner wrote:
> >
> > is there any reason why kfree() takes a const pointer just to degrade it
> > with the call to slab_free()/__cache_free() again? The promise that the
> > pointee is not modified is just bogus in this case, anyway, isn't it?
>
> "const" has *never* been about the thing not being modified. Forget all
> that claptrap. C does not have such a notion.
>
> "const" is a pointer type issue, and is meant to make certain mis-uses
> more visible at compile time. It has *no* other meaning, and anybody who
> thinks it has is just setting himself up for problems.
I totally agree with the above.
>
> In the particular case of kfree(), the pointer argument *should* be const,
> and the fact that the C library gets this wrong is not the kernels
> problem, it's a problem with the C library.
Here, I'm not so sure.
>
> Why?
>
> - From a very obvious and very *real* caller perspective, "free()" really
> doesn't change the thing the pointer points to. It does something
> totally different: it makes the *pointer* itself invalid.
I'm OK with this.
>
> In other words, if you think that "kfree()" changed the thing you
> free'd, you're simply wrong. It did no such thing. The memory is 100%
> the same, it's just that you cannot access it any more, and if you try,
> you'll get somebody elses memory.
This too.
>
> In other words, "kfree()" can be const.
Err, not sure.
>
> - Anything that *can* take a const pointer should always do so.
>
> Why? Because we want the types to be as tight as possible, and normal
> code should need as few casts as possible.
OK
>
> Here's a question for you: let's say that you have a structure that
> has a member that is never changed. To make that obvious, and to allow
> the compiler to warn about mis-use of a pointer, the structure should
> look something like
>
> struct mystruct {
> const char *name;
> ..
>
> and let's look at what happens if the allocation of that const thing is
> dynamic.
>
> The *correct* way to do that is:
>
> char *name = kmalloc(...)
> /* Fill it in */
> snprintf(name, ...)
> mystruct->name = name;
>
> and there are no casts anywhere, and you get exactly the semantics you
> want: "name" itself isn't constant (it's obviously modified), but at
> the same time the type system makes it very clear that trying to change
> it through that mystruct member pointer is wrong.
>
> How do you free it?
>
> That's right, you do:
>
> kfree(mystruct->name);
This is where I disagree. If a struct has a constant pointer to it, then
the usage of that pointer by the struct should never modify it. If I
need to allocate memory for a name to a struct, I would not expect that
struct to ever free it.
Let's use your example. I'll assume that the struct was created by some
constructor and the destructor freed it. I'd argue the correct way would
be to have the kfree with a typecast.
Why?
- const pointers (especially strings) should be able to point to
static data. One thing that we would like to avoid is:
mystruct->name = "myobj";
...
kfree(mystruct->name);
- really, kfree should match kmalloc for types. What kmalloc returns
should be what kfree accepts. Passing in a const pointer to kfree
*should* be a red flag that something might not be right.
>
> and this is why "kfree()" should take a const pointer. If it doesn't,
> you have to add an *incorrect* and totally useless cast to code that
> was correct.
char *name = kmalloc(...);
...
mystruct->name = name; /* this is an implicit cast */
So adding a cast to kfree isn't incorrect. C automatically casts
name to a const pointer, which means if we want to free it, then
we should cast it back.
>
> So never believe that "const" is some guarantee that the memory under the
> pointer doesn't change. That is *never* true. It has never been true in
> C, since there can be arbitrary pointer aliases to that memory that aren't
> actually const. If you think "const *p" means that the memory behind "p"
> is immutable, you're simply wrong.
Again, I totally agree with the above.
>
> Anybody who thinks that kfree() cannot (or should not) be const doesn't
> understand the C type system.
OK, I think that kfree should not be const, but not because of the
explanation that you gave, but because of the C type system in general.
kfree should match the kmalloc type. We don't declare
const void *kmalloc(...)
so we shouldn't do the same with kfree. If you assign a const pointer to
something from kmalloc, C implicitly does the cast. This doesn't mean
that we should ignore doing the cast back in kfree. Especially since
this could help us avoid the kfree("mystring") issue.
Just my $0.02
-- Steve
Hi,
Christoph Lameter <[email protected]> writes:
> On Wed, 16 Jan 2008, Johannes Weiner wrote:
>
>> So if I got it right and you actually modify the memory you only got a
>> const pointer to, you reach a level where you _have to_ break this
>> policy and cast to a non-const pointer, as it is currently done in
>> kfree(). No?
>
> Correct and we have gcc 4.2 currently spitting out warnings because of
> casting to non const. Any idea how to convince gcc that this is okay?
Two dirty hacks where gcc at least does not complain:
void *y = (void *)x;
and then pass y, or passing
*(void **)&x
directly.
Both approaches seem just too ugly to silence a bogus warning.
Hannes
Hi,
Christoph Lameter <[email protected]> writes:
> On Wed, 16 Jan 2008, Johannes Weiner wrote:
>
>> So if I got it right and you actually modify the memory you only got a
>> const pointer to, you reach a level where you _have to_ break this
>> policy and cast to a non-const pointer, as it is currently done in
>> kfree(). No?
>
> Correct and we have gcc 4.2 currently spitting out warnings because of
> casting to non const. Any idea how to convince gcc that this is okay?
Just for the record, this really seems to be a gcc bug, I can not
explain this otherwise:
$ cat test.c
#include <stdio.h>
static void test(void *p)
{
char *v = p;
puts(v);
}
int main(void)
{
const char foo[] = "foo";
test((void *)foo);
return 0;
}
$ gcc -Wall test.c -o test
$ gcc -Wall -O test.c -o test
test.c: In function 'main':
test.c:12: warning: passing argument 1 of 'test' discards qualifiers from pointer target type
$ gcc -Wall -O -fno-inline test.c -o test
$
gcc is version 4.2.2.
Hannes
On Wed, 16 Jan 2008, Johannes Weiner wrote:
>
> Okay, I understood that now. A const qualifier just forbids modifying
> the underlying memory _through this particular pointer_, right?
Yes, exactly.
> In the case of slub's kfree(), which takes a const pointer, you pass it
> on to slab_free() which actually _DOES_ modification of the underlying
> memory when linking the object to the freelist (as far as I understood
> the code).
First off, the whole concept of "underlying memory" is all about the fact
that pointers point not to separate objects with its own existence, but
point to a shared resource ("memory") that can be accessed many different
ways.
So immediately when you talk about "underlying memory", you need to
realize that now you're not talking about "that pointer" any more, but you
are talking about the stuff that *other* pointers can access.
And the whole (and *only*) point of a memory allocator is turning that
amorphous notion of "infinitely aliasable underlying memory" model into
more of a "C pointer to separate objects" model.
That *is* what a memory allocator does.
> So if I got it right and you actually modify the memory you only got a
> const pointer to
No. YOU DO NOT.
You *invalidate* the pointer. Then, the memory allocator releases that
portion of the "infinitely aliasable underlying memory", but that means
that THAT POINTER NO LONGER EXISTS ON A C LEVEL.
So no, you don't "modify the memory you only got a const pointer to".
What you do is that the memory allocator
- keeps track of the NON-const memory that it always had
- it sees the const pointer you gave it, and uses that pointer to look up
ITS OWN DATA STRUCTURES.
- it then accesses those memory locations using its own pointers.
The "const pointer" you passed to kfree() simply no longer exists. The
object it pointed to has gone away. That particular pointer simply isn't
valid any more.
But the memory management code, which maintains its own structures for
what it has allocated and not allocated, can (and will) generally use the
knowledge *it* has about its own pointers to modify its own data
structures.
The fact that they (obviously) alias in memory is irrelevant. It has no
meaning for the "const void *" you passed in. That pointer is not usable
for the caller any more.
And no, this is not just a semantic argument. It's a real issue. The
pointer you pass in to kfree() is obviously used to *find* the data
structures that the memory allocator maintains, and there is almost
inevitably a rather direct mapping between the passed-in pointers and the
data structures that a memory manager maintains itself, but they really
are different.
So to take a kfree() a pointer, the memory manager will use the "binary
value" of that pointer to find its own data structures, and sometimes the
values are so closely related that it ends up being a direct cast
(possibly with an offset), but the end result of that direct cast really
is now the memory management internal data, not the original pointer.
A real example of this is the actual kfree() implementation in the kernel
(let's take the one from SLAB):
struct kmem_cache *c;
..
c = virt_to_cache(objp);
which then actually ends up doing a
struct page *page = virt_to_head_page(obj);
return page_get_cache(page);
which literally does math on the binary representation of the pointer to
look up the backing store values (because the memory manager by definition
is the thing that knows how the low-level representation of pointers
actually works).
So now the kernel has turned that user-supplied pointer into ITS OWN data
structures, that contain the NON-CONST aliases for the data.
(It will actually use the const pointer to further get offsets into those
things, of course, so it's several levels of indirection off that pointer)
And yes, then it will write to those non-const aliases, but that's really
no different from the same old thing: the pointer *you* passed kmalloc()
may be "const", but the memory manager itself has its own data structures
to keep track of *its* aliases to the same memory, and those are the ones
that are actually used to re-use the memory for others (and poison it
etc).
So the particular pointer you passed in is "const". But that never *ever*
guarantees that there aren't other non-const pointers around to be used
that can modify the memory. And the memory manegement definitely has all
those aliased pointers around - it's the whole *point* of memory
management.
Linus
On Wed, 16 Jan 2008, Christoph Lameter wrote:
>
> Correct and we have gcc 4.2 currently spitting out warnings because of
> casting to non const. Any idea how to convince gcc that this is okay?
Either don't use a broken compiler (casting a const pointer to a non-const
is definitely not a bug), or cast to "unsigned long" (if it still
complains, now the compiler is not just stupid, it's broken).
The whole point of memory management is that we know how pointers work,
and understand that they have a *bit* representation, not just the C
semantics.
Linus
On Thu, 17 Jan 2008, David Schwartz wrote:
>
> Which does change the thing the pointer points to. It goes from being a
> valid object to not existing.
No. It's the *pointer* that is no longer valid.
There's definitely a difference between "exists and is changed" and
"doesn't exist any more".
> How is ceasing to exist not a change?
It's not a change to the data behind it, it's a change to the *metadata*.
Which is somethign that "const" doesn't talk about at all.
> > Why? Because we want the types to be as tight as possible, and normal
> > code should need as few casts as possible.
>
> Right, and that's why you are wrong.
No, it's why I'm right.
"kmalloc/kfree" (or any memory manager) by definition has to play games
with pointers and do things like cast them. But the users shouldn't need
to, not for something like this.
> No, it's both correct and useful. This code is the exception to a rule. The
> rule is that the object remain unchanged and this violates that rule.
No.
You are continuing to make the mistake that you think that "const" means
that the memory behind the pointer is not going to change.
Why do you make that mistake, when it is PROVABLY NOT TRUE!
Try this trivial program:
int main(int argc, char **argv)
{
int i;
const int *c;
i = 5;
c = &i;
i = 10;
return *c;
}
and realize that according to the C rules, if it returns anything but 10,
the compiler is *buggy*.
The fact is, that in spite of us having a "const int *", the data behind
that pointer may change.
So it doesn't matter ONE WHIT if you pass in a "const *" to "kfree()": it
does not guarantee that the data doesn't change, because the object you
point to has other pointers pointing to it.
This isn't worth discussing. It's really simple: a conforming program
CANNOT POSSIBLY TELL whether "kfree()" modified the data or not. As such,
AS FAR AS THE PROGRAM IS CONCERNED, kfree() takes a const pointer, and the
rule that "if it can be considered const, it should be marked const" comes
and says that kfree() should take a const pointer.
In other words - anythign that could ever disagree with "const *" is BY
DEFINITION buggy.
It really is that simple.
Linus
> On Thu, 17 Jan 2008, David Schwartz wrote:
> > Which does change the thing the pointer points to. It goes from being a
> > valid object to not existing.
> No. It's the *pointer* that is no longer valid.
The pointer is no longer valid because the object it pointed to no longer
exists. The pointer is also no longer valid, but that is not the end of the
story.
> There's definitely a difference between "exists and is changed" and
> "doesn't exist any more".
> > How is ceasing to exist not a change?
> It's not a change to the data behind it, it's a change to the *metadata*.
> Which is somethign that "const" doesn't talk about at all.
It doesn't matter what has changed. All that matters is whether this is
something we normally want to happen to a const pointer or whether doing
this to a const pointer is not normal.
> > > Why? Because we want the types to be as tight as possible,
> > > and normal
> > > code should need as few casts as possible.
> >
> > Right, and that's why you are wrong.
>
> No, it's why I'm right.
> "kmalloc/kfree" (or any memory manager) by definition has to play games
> with pointers and do things like cast them. But the users shouldn't need
> to, not for something like this.
If you don't like having to cast, don't use 'const'. But if you use 'const',
you have to cast when you mean to do something that you would like to be
warned about if you do it by accident.
> > No, it's both correct and useful. This code is the exception to
> a rule. The
> > rule is that the object remain unchanged and this violates that rule.
> No.
>
> You are continuing to make the mistake that you think that "const" means
> that the memory behind the pointer is not going to change.
No, that's not what it means. It has nothing to do with memory. It has to do
with logical state.
> Why do you make that mistake, when it is PROVABLY NOT TRUE!
I don't. You do, because you argue 'kfree' can be const because it doesn't
change the memory. The change in the memory is meaningless, the change in
the logical state of the object is what matters.
> Try this trivial program:
>
> int main(int argc, char **argv)
> {
> int i;
> const int *c;
>
> i = 5;
> c = &i;
> i = 10;
> return *c;
> }
>
> and realize that according to the C rules, if it returns anything but 10,
> the compiler is *buggy*.
>
> The fact is, that in spite of us having a "const int *", the data behind
> that pointer may change.
I don't know what you think this example proves. Nobody is arguing that so
long as one const pointer to an object exists, no code anywhere should ever
be able to change it.
All I'm saying is that changing the logical state of an object *through* a
const pointer is unusual. You should need a cast to do this because that's
the only way to get a warning if you do it by mistake.
> So it doesn't matter ONE WHIT if you pass in a "const *" to "kfree()": it
> does not guarantee that the data doesn't change, because the object you
> point to has other pointers pointing to it.
Right, nobody said this was about guaranteeing that data doesn't change.
> This isn't worth discussing. It's really simple: a conforming program
> CANNOT POSSIBLY TELL whether "kfree()" modified the data or not.
But that's exactly what doesn't matter. As you've said at least twice now,
it has nothing to do with changing the data. It has to do with changing the
logical state of the object. That's what you're not supposed to do through a
'const' pointer.
> As such,
> AS FAR AS THE PROGRAM IS CONCERNED, kfree() takes a const
> pointer, and the
> rule that "if it can be considered const, it should be marked
> const" comes
> and says that kfree() should take a const pointer.
That's crazy.
> In other words - anythign that could ever disagree with "const *" is BY
> DEFINITION buggy.
>
> It really is that simple.
I think you may be the only person in the world who thinks so.
DS
On Thu, 17 Jan 2008, David Schwartz wrote:
>
> No, that's not what it means. It has nothing to do with memory. It has to do
> with logical state.
Blah. That's just your own made-up explanation of what you think "const"
should mean. It has no logical background or any basis in the C language.
"const" has nothing to do with "logical state". It has one meaning, and
one meaning only: the compiler should complain if that particular type is
used to do a write access.
It says nothing at all about the "logical state of the object". It cannot,
since a single object can - and does - have multiple pointers to it.
So your standpoint not only has no relevant background to it, it's also
not even logically consistent.
Linus
> On Thu, 17 Jan 2008, David Schwartz wrote:
> > No, that's not what it means. It has nothing to do with memory.
> > It has to do
> > with logical state.
> Blah. That's just your own made-up explanation of what you think "const"
> should mean. It has no logical background or any basis in the C language.
To some extent, I agree. You can use "const" for pretty much any reason.
It's just a way to say that you have a pointer and you would like an error
if certain things are done with it.
You could use it to mean anything you want it to mean. The most common use,
and the one intended, is to indicate that an object's logical state will not
be changed through that pointer.
> "const" has nothing to do with "logical state". It has one meaning, and
> one meaning only: the compiler should complain if that particular type is
> used to do a write access.
Right, exactly.
> It says nothing at all about the "logical state of the object".
> It cannot,
> since a single object can - and does - have multiple pointers to it.
You are the only one who has suggested it has anything to do with changes
through other pointers or in other ways. So you are arguing against only
yourself here.
Nobody has said it has anything to do with anything but operations through
that pointer.
> So your standpoint not only has no relevant background to it, it's also
> not even logically consistent.
Actually, that is true of your position. On the one hand, you defend it
because kfree does not change the data. On the other hand, you claim that it
has nothing to do with whether or not the data is changed.
The normal use of "const" is to indicate that the logical state of the
object should not be changed through that pointer. The 'kfree' function
changes the logical state of the object. So, logically, 'kfree' should not
be const.
The usefulness of "const" is that you get an error if you unexpectedly
modify something you weren't expected to modify. If you are 'kfree'ing an
object that is supposed to be logically immutable, you should be made to
indicate that you are aware the object is logically immutable.
Simply put, you you have to cast in any case where you mean to do something
that you want to get an error in if you do not cast. I would like to get an
error if I call 'kfree' through a const pointer, because that often is an
error. I may have a const pointer because my caller still plans to use the
object.
Honestly, I find your position bizarre.
DS
On Thu, 17 Jan 2008, David Schwartz wrote:
>
> > "const" has nothing to do with "logical state". It has one meaning, and
> > one meaning only: the compiler should complain if that particular type is
> > used to do a write access.
>
> Right, exactly.
So why do you complain?
kfree() literally doesn't write to the object.
> You are the only one who has suggested it has anything to do with changes
> through other pointers or in other ways. So you are arguing against only
> yourself here.
No, I'm saying that "const" has absolutely *zero* meaning on writes to an
object through _other_ pointers (or direct access) to the object.
And you're seemingly not understanding that *lack* of meaning.
kfree() doesn't do *squat* to the object pointed to by the pointer it is
passed. It only uses it to look up its own data structures, of which the
pointer is but a small detail.
And those other data structures aren't constant.
> Nobody has said it has anything to do with anything but operations through
> that pointer.
.. and I'm telling you: kfree() does *nothing* conceptually through that
pointer. No writes, and not even any reads! Which is exactly why it's
const.
The only thing kfree does through that pointer is to update its own
concept of what memory it has free.
Now, what it does to its own free memory is just an implementation detail,
and has nothing what-so-ever to do with the pointer you passed it.
See?
Linus
> On Thu, 17 Jan 2008, David Schwartz wrote:
> > > "const" has nothing to do with "logical state". It has one
> > > meaning, and
> > > one meaning only: the compiler should complain if that
> > > particular type is
> > > used to do a write access.
> >
> > Right, exactly.
> So why do you complain?
>
> kfree() literally doesn't write to the object.
Because the object ceases to exist. However, any modification requires write
access, whether or not that modification is a write.
> > You are the only one who has suggested it has anything to do
> > with changes
> > through other pointers or in other ways. So you are arguing against only
> > yourself here.
> No, I'm saying that "const" has absolutely *zero* meaning on writes to an
> object through _other_ pointers (or direct access) to the object.
Nobody disagrees with that.
> And you're seemingly not understanding that *lack* of meaning.
No, I understand that.
> kfree() doesn't do *squat* to the object pointed to by the pointer it is
> passed. It only uses it to look up its own data structures, of which the
> pointer is but a small detail.
> And those other data structures aren't constant.
Nonsense. The 'kfree' function *destroys* the object pointer to by the
pointer. How can you describe that as not doing anything to the object?
> > Nobody has said it has anything to do with anything but
> > operations through
> > that pointer.
> .. and I'm telling you: kfree() does *nothing* conceptually through that
> pointer. No writes, and not even any reads! Which is exactly why it's
> const.
It destroys the object the pointer points to. Destroying an object requires
write access to it.
> The only thing kfree does through that pointer is to update its own
> concept of what memory it has free.
That is not what it does, that is how it does it. What it does is destroy
the object.
> Now, what it does to its own free memory is just an
> implementation detail,
> and has nothing what-so-ever to do with the pointer you passed it.
I agree, except that it destroys the object the pointer points to.
> See?
I now have a much better understanding of what you're saying, but I still
think it's nonsense.
1) An operation that modifies the logical state of an object should not
normally be done through a 'const' pointer. The reason you make a pointer
'const' is to indicate that this pointer should not be used to change the
logical state of the object pointed to.
2) The 'kfree' operation changes the logical state of the object pointed to,
as the object goes from existent to non-existent.
3) It is most useful for 'kfree' to be non-const because destroying an
object through a const pointer can easily be done in error. One of the
reasons you provide a const pointer is because you need the function you
pass the pointer to not to modify the object. Since this is an unusual
operation that could be an error, it is logical to force the person doing it
to clearly indicate that he knows the pointer is const and that he knows it
is right anyway.
I'm curious to hear how some other people on this feel. You are the first
competent coder I have *ever* heard make this argument.
By the way, I disagree with your metadata versus data argument. I would
agree that a function that changes only an object's metadata could be done
through a const pointer without needed a cast. A good example would be a
function that updates a "last time this object was read" variable.
However, *destroying* an object is not a metadata operation -- it destroys
the data as well. This is kind of a philosophical point, but an object does
not have a "does this object exist" piece of metadata. If an object does not
exist, it has no data. So destroying an object destroys the data and is thus
a write/modification operation on the data.
DS
Linus Torvalds wrote:
>
> On Thu, 17 Jan 2008, David Schwartz wrote:
>>> "const" has nothing to do with "logical state". It has one meaning, and
>>> one meaning only: the compiler should complain if that particular type is
>>> used to do a write access.
>> Right, exactly.
>
> So why do you complain?
>
> kfree() literally doesn't write to the object.
>
>> You are the only one who has suggested it has anything to do with changes
>> through other pointers or in other ways. So you are arguing against only
>> yourself here.
>
> No, I'm saying that "const" has absolutely *zero* meaning on writes to an
> object through _other_ pointers (or direct access) to the object.
Hints: "restrict" is the C99 keyword for such requirement (or better
"const restrict")
BTW I think C use non const free as a BIG warning about not to be
to "smart" on optimization.
ciao
cate
And to demostrate that Linus is not the only person
with this view, I copy some paragraphs from C99 rationale
(you can find standard, rationale and other documents
in http://clc-wiki.net/wiki/C_standardisation:ISO )
Page 75 of C99 rationale:
Type qualifiers were introduced in part to provide greater control over optimization. Several
important optimization techniques are based on the principle of "cacheing": under certain
circumstances the compiler can remember the last value accessed (read or written) from a
location, and use this retained value the next time that location is read. (The memory, or
"cache", is typically a hardware register.) If this memory is a machine register, for instance, the
code can be smaller and faster using the register rather than accessing external memory.
The basic qualifiers can be characterized by the restrictions they impose on access and
cacheing:
const No writes through this lvalue. In the absence of this qualifier, writes may occur
through this lvalue.
volatile No cacheing through this lvalue: each operation in the abstract semantics must
be performed (that is, no cacheing assumptions may be made, since the location
is not guaranteed to contain any previous value). In the absence of this qualifier,
the contents of the designated location may be assumed to be unchanged except
for possible aliasing.
restrict Objects referenced through a restrict-qualified pointer have a special
association with that pointer. All references to that object must directly or
indirectly use the value of this pointer. In the absence of this qualifier, other
pointers can alias this object. Cacheing the value in an object designated through
a restrict-qualified pointer is safe at the beginning of the block in which the
pointer is declared, because no pre-existing aliases may also be used to reference
that object. The cached value must be restored to the object by the end of the
block, where pre-existing aliases again become available. New aliases may be
formed within the block, but these must all depend on the value of the
restrict-qualified pointer, so that they can be identified and adjusted to refer
to the cached value. For a restrict-qualified pointer at file scope, the block
is the body of main.
ciao
cate
On Thursday 17 January 2008 11:51:49 pm Giacomo Catenazzi wrote:
> Linus Torvalds wrote:
> > No, I'm saying that "const" has absolutely *zero* meaning on writes to an
> > object through _other_ pointers (or direct access) to the object.
>
> Hints: "restrict" is the C99 keyword for such requirement (or better
> "const restrict")
The restrict keyword controls aliasing, to be exact. And I'm skeptical that
inserting const there would do anything at all.
> BTW I think C use non const free as a BIG warning about not to be
> to "smart" on optimization.
I must ask what relationship you think the const keyword has to compiler
optimizations. I know of none, and I've yet to see that keyword cause any
difference in the resulting assembly. It forces you to make your code clean
and well-structured, but that's about it.
Of course, it would be an interesting experiment to potentially redefine the
const keyword to have stronger semantics, such as having the compiler assume
that a function taking a const pointer argument will not modify the memory
the pointer points to, and thus saving itself a memory load in the caller
after the function executes, as long as the data is not global. I imagine
that this would lead to some simple and measurable optimizations, all the
while (this is where I get into hand-waving territory) breaking a minimum
amount of code in current existence.
But that is emphatically not how C is currently defined, and you're basically
inventing an entirely new language... C2009 perhaps? :-)
-- Vadim Lobanov
On Thu, Jan 17, 2008 at 01:25:39PM -0800, Linus Torvalds wrote:
...
> Why do you make that mistake, when it is PROVABLY NOT TRUE!
>
> Try this trivial program:
>
> int main(int argc, char **argv)
> {
> int i;
> const int *c;
>
> i = 5;
> c = &i;
> i = 10;
> return *c;
> }
>
> and realize that according to the C rules, if it returns anything but 10,
> the compiler is *buggy*.
That's not how this works (as we obviously agree).
Please consider a rewrite of your example, demonstrating the usefulness and
proper application of const pointers:
extern foo(const int *);
int main(int argc, char **argv)
{
int i;
i = 5;
foo(&i);
return i;
}
Now, if the program returns anything else than 5, it means someone cast away
const, which is generally considered a bad idea in most other software
projects, for this very reason.
*That* is the purpose of const pointers.
Besides, for most debugging-enabled free() implementations, free() does indeed
touch the memory pointed to by its argument, which makes giving it a const
pointer completely bogus except for a single potential optimized special-case
where it might actually not touch the memory.
--
/ jakob
Jakob Oestergaard wrote:
> On Thu, Jan 17, 2008 at 01:25:39PM -0800, Linus Torvalds wrote:
> ...
>> Why do you make that mistake, when it is PROVABLY NOT TRUE!
>>
>> Try this trivial program:
>>
>> int main(int argc, char **argv)
>> {
>> int i;
>> const int *c;
>>
>> i = 5;
>> c = &i;
>> i = 10;
>> return *c;
>> }
>>
>> and realize that according to the C rules, if it returns anything but 10,
>> the compiler is *buggy*.
>
> That's not how this works (as we obviously agree).
>
> Please consider a rewrite of your example, demonstrating the usefulness and
> proper application of const pointers:
>
> extern foo(const int *);
>
> int main(int argc, char **argv)
> {
> int i;
>
> i = 5;
> foo(&i);
> return i;
> }
>
> Now, if the program returns anything else than 5, it means someone cast away
> const, which is generally considered a bad idea in most other software
> projects, for this very reason.
>
> *That* is the purpose of const pointers.
"restrict" exists for this reason. const is only about lvalue.
You should draw a line, not to make C more complex!
Changing the name of variables in your example:
extern print_int(const int *);
int main(int argc, char **argv)
{
extern int errno;
errno = 0;
print_int(&i);
return errno;
}
print_int() doesn't know that errno is also the argument.
and this compilation unit doesn't know that print_int() will
modify errno.
Ok, I changed int to extern int, but you see the point?
Do you want complex rules about const, depending on
context (extern, volatile,...) ?
ciao
cate
Giacomo Catenazzi wrote:
> const No writes through this lvalue. In the absence of this qualifier, writes may occur
> through this lvalue.
>
> volatile No cacheing through this lvalue: each operation in the abstract semantics must
> be performed (that is, no cacheing assumptions may be made, since the location
> is not guaranteed to contain any previous value). In the absence of this qualifier,
> the contents of the designated location may be assumed to be unchanged except
> for possible aliasing.
Well, I'm still wondering if there is not something dangerous or weird about
declaring the argument of kfree() to be const...
Since the content of the referenced object is unlikely to be declared volatile, the
compiler should be allowed to assume that its content was not changed, except
for possible aliasing. But what happens if the compiler can also prove there is
no aliasing? In that case, he should be allowed to assume that the content
pointed to was not modified at all, right?
Consider the following code :
struct something {
int i;
};
...
struct something *s1, *s2;
extern int val;
s1 = kmalloc(sizeof(*s1), SOME_FLAGS);
if (s1 == NULL)
return -ENOMEM;
s1->i = do_something();
do_some_other_thing();
s2 = kmalloc(sizeof(*s2), SOME_FLAGS);
if (s2 == NULL){
val = s1->i; /* XXX */
kfree(s1);
return -ENOMEM;
}
Fortunately, kmalloc is not declared with attribute malloc in the kernel,
so there should be no problem, but if it were (and, actually, I've not found
why it wasn't), the compiler would be able to tell that *s1 *cannot*
be aliased, and therefore decide to move val = s1->i *after* having
called kfree(). In that case, we would clearly have a bug...
So, although this should currently work, code which breaks if you do
a legitimate modification somewere else looks quite dangerous to me.
Or maybe there is a rationale for never declaring kmalloc to have the
attribute malloc in the first place...
Or is there simply something that I still don't understand, Giacomo?
Cheers,
Emmanuel Colbus
On 2008.01.18 10:48:26 +0100, Jakob Oestergaard wrote:
> On Thu, Jan 17, 2008 at 01:25:39PM -0800, Linus Torvalds wrote:
> ...
> > Why do you make that mistake, when it is PROVABLY NOT TRUE!
> >
> > Try this trivial program:
> >
> > int main(int argc, char **argv)
> > {
> > int i;
> > const int *c;
> >
> > i = 5;
> > c = &i;
> > i = 10;
> > return *c;
> > }
> >
> > and realize that according to the C rules, if it returns anything but 10,
> > the compiler is *buggy*.
>
> That's not how this works (as we obviously agree).
>
> Please consider a rewrite of your example, demonstrating the usefulness and
> proper application of const pointers:
>
> extern foo(const int *);
>
> int main(int argc, char **argv)
> {
> int i;
>
> i = 5;
> foo(&i);
> return i;
> }
>
> Now, if the program returns anything else than 5, it means someone cast away
> const, which is generally considered a bad idea in most other software
> projects, for this very reason.
Not at all.
#include <stdio.h>
#include <stdlib.h>
char *lookup[5];
const char *get()
{
*(*lookup = malloc(1)) = '1';
return *lookup;
}
void set(const char *d, char val)
{
for (int i = 0; i < 5; ++i)
if (lookup[i] == d)
*(lookup[i]) = val;
}
int main()
{
const char *p = get();
printf("%c\n", *p);
set(p, '2');
printf("%c\n", *p);
return 0;
}
Do you see anything that casts the const away? No? Me neither. Still,
the memory that p points to was changed, because there was another
pointer and that was not const.
> *That* is the purpose of const pointers.
The only thing that const can tell you is that you should not modify the
value _yourself_, using that pointer _directly_. It's somewhat like a
soft "half" protected/private specifier. You may read this value, but if
you want to write to it, please use the setter function I provide for
you. Because that setter function might do some special stuff, like
counting how often that value was written.
And accepting a pointer to a const as an argument does _only_ say: It's
ok to call this function if you only received a pointer to a const, the
function does the Right Thing for such pointers. It does not guarantee
at all, that the function won't change the memory the pointer is
pointing to. Take a set of functions that manage memory for foo objects:
const struct foo *get(someIdentifier);
struct foo *makeWritable(const struct foo *);
void free(const struct foo *);
get() returns a pointer to a foo object, and it might return a pointer
to a _shared_ instance. Obviously it should make the pointer const, the
caller should not modify the shared instance.
makeWritable() accepts a pointer to a const foo, because you generally
want to pass it such a pointer to get a non-const one instead. The
function might just use ref-counting and see if it needs to create a
copy returning a pointer to points to a different location in memory or
if it can just return its _internal_ _non-const_ pointer. No casts!
free() also accepts a pointer to a const foo, because you obviously want
to be able to free the shared stuff, too. It just happens to not always
go away immediately, because there's some ref-counting going on. But you
are _still_ invalidating your const pointer. You lost all rights to
using it, because you _gave it up_ when you called free(). An important
part of free() is to _invalidate_ the _pointer_. Whether or not the
object it pointed to was modified or not doesn't matter at all, because
you have no valid means of accessing it _anyway_, it's totally out of
scope.
Now I can hear you screaming "But that's refcounting! That's totally
different!". It's _not_. Just "pretend" that get() cannot return the
same thing twice, then your ref-counting only goes up to one and boom,
you've just reinvented malloc/free, with a const pointer being passed to
free().
Passing a pointer to free invalidates that pointer, and all pointers
that share the same value. And then, when no valid ways to access the
object are left, free() is free to use the memory in any way it wants.
Without any stricht need to use the const pointer to perform any writes.
That might be an optimization, but you can do without.
What you're arguing about is actually that you don't want anyone to be
able to use a const pointer to invalidate any other pointer, not that
you don't want an object to be changed through a const pointer, because
that is simply not what happens.
If you want to restrict the set of pointers that can be invalidated by
an other pointer, you'll have to use something else because const does
not talk about invalidating aliasing pointers.
Bj?rn
Giacomo Catenazzi wrote:
> And to demostrate that Linus is not the only person
> with this view, I copy some paragraphs from C99 rationale
> (you can find standard, rationale and other documents
> in http://clc-wiki.net/wiki/C_standardisation:ISO )
>
> Page 75 of C99 rationale:
>
> Type qualifiers were introduced in part to provide greater control over optimization. Several
> important optimization techniques are based on the principle of "cacheing": under certain
> circumstances the compiler can remember the last value accessed (read or written) from a
> location, and use this retained value the next time that location is read. (The memory, or
> "cache", is typically a hardware register.) If this memory is a machine register, for instance, the
> code can be smaller and faster using the register rather than accessing external memory.
> The basic qualifiers can be characterized by the restrictions they impose on access and
> cacheing:
>
> const No writes through this lvalue. In the absence of this qualifier, writes may occur
> through this lvalue.
>
I'd say this implies the exact opposite. It almost sounds like the
compiler is free to change:
void foo(const int *x);
foo(x);
printf("%d", x);
to:
void foo(const int *x);
printf("%d", x);
foo(x);
especially if it can prove that the pointer to x doesn't otherwise
escape or that foo doesn't call anything that could see the pointer (and
given that gcc has special magical markings for malloc, one way this
could be "proven" is to have x be some freshly malloced object.
If foo is kfree, then the above transformation is clearly invalid.
(Note that this isn't just a problem for optimizers -- a programmer
might expect that passing a pointer to a function that takes a const
pointer argument does not, in and of itself, change the pointed-to
value. Given that const certainly does not mean that no one else
changes the object, I'm not sure what else it could mean. kfree does
not have either property, so I'm don't think it makes sense for it to
take a const argument.)
--Andy
> volatile No cacheing through this lvalue: each operation in the abstract semantics must
> be performed (that is, no cacheing assumptions may be made, since the location
> is not guaranteed to contain any previous value). In the absence of this qualifier,
> the contents of the designated location may be assumed to be unchanged except
> for possible aliasing.
>
> restrict Objects referenced through a restrict-qualified pointer have a special
> association with that pointer. All references to that object must directly or
> indirectly use the value of this pointer. In the absence of this qualifier, other
> pointers can alias this object. Cacheing the value in an object designated through
> a restrict-qualified pointer is safe at the beginning of the block in which the
> pointer is declared, because no pre-existing aliases may also be used to reference
> that object. The cached value must be restored to the object by the end of the
> block, where pre-existing aliases again become available. New aliases may be
> formed within the block, but these must all depend on the value of the
> restrict-qualified pointer, so that they can be identified and adjusted to refer
> to the cached value. For a restrict-qualified pointer at file scope, the block
> is the body of main.
>
> ciao
> cate
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
Giacomo Catenazzi wrote:
> And to demostrate that Linus is not the only person
> with this view, I copy some paragraphs from C99 rationale
> (you can find standard, rationale and other documents
> in http://clc-wiki.net/wiki/C_standardisation:ISO )
>
> Page 75 of C99 rationale:
>
> Type qualifiers were introduced in part to provide greater control over optimization. Several
> important optimization techniques are based on the principle of "cacheing": under certain
> circumstances the compiler can remember the last value accessed (read or written) from a
> location, and use this retained value the next time that location is read. (The memory, or
> "cache", is typically a hardware register.) If this memory is a machine register, for instance, the
> code can be smaller and faster using the register rather than accessing external memory.
> The basic qualifiers can be characterized by the restrictions they impose on access and
> cacheing:
>
> const No writes through this lvalue. In the absence of this qualifier, writes may occur
> through this lvalue.
>
I'd say this implies the exact opposite. It almost sounds like the
compiler is free to change:
void foo(const int *x);
foo(x);
printf("%d", x);
to:
void foo(const int *x);
printf("%d", x);
foo(x);
especially if it can prove that the pointer to x doesn't otherwise
escape or that foo doesn't call anything that could see the pointer (and
given that gcc has special magical markings for malloc, one way this
could be "proven" is to have x be some freshly malloced object.
If foo is kfree, then the above transformation is clearly invalid.
(Note that this isn't just a problem for optimizers -- a programmer
might expect that passing a pointer to a function that takes a const
pointer argument does not, in and of itself, change the pointed-to
value. Given that const certainly does not mean that no one else
changes the object, I'm not sure what else it could mean. kfree does
not have either property, so I'm don't think it makes sense for it to
take a const argument.)
--Andy
> volatile No cacheing through this lvalue: each operation in the abstract semantics must
> be performed (that is, no cacheing assumptions may be made, since the location
> is not guaranteed to contain any previous value). In the absence of this qualifier,
> the contents of the designated location may be assumed to be unchanged except
> for possible aliasing.
>
> restrict Objects referenced through a restrict-qualified pointer have a special
> association with that pointer. All references to that object must directly or
> indirectly use the value of this pointer. In the absence of this qualifier, other
> pointers can alias this object. Cacheing the value in an object designated through
> a restrict-qualified pointer is safe at the beginning of the block in which the
> pointer is declared, because no pre-existing aliases may also be used to reference
> that object. The cached value must be restored to the object by the end of the
> block, where pre-existing aliases again become available. New aliases may be
> formed within the block, but these must all depend on the value of the
> restrict-qualified pointer, so that they can be identified and adjusted to refer
> to the cached value. For a restrict-qualified pointer at file scope, the block
> is the body of main.
>
> ciao
> cate
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
On Fri, Jan 18, 2008 at 12:47:01PM +0100, Giacomo A. Catenazzi wrote:
...
> "restrict" exists for this reason. const is only about lvalue.
You think that I try to put more meaning into const than I do - but I don't.
Please read what I wrote, not what you want to think I wrote.
I agree that if I said what you seem to imply I said, then I would have been
wrong. But I didn't so I'm not ;)
--
/ jakob
On Fri, Jan 18, 2008 at 02:31:16PM +0100, Bj?rn Steinbrink wrote:
...
>
> Do you see anything that casts the const away? No? Me neither. Still,
> the memory that p points to was changed, because there was another
> pointer and that was not const.
*another* being key here.
>
> > *That* is the purpose of const pointers.
>
> The only thing that const can tell you is that you should not modify the
> value _yourself_, using that pointer _directly_.
Which is pretty damn useful.
Think about it. Don't you ever use const? Is it ever only in the way?
...
{snip long explanation about how one can avoid the benefits of const, without
using casts}
...
> If you want to restrict the set of pointers that can be invalidated by
> an other pointer, you'll have to use something else because const does
> not talk about invalidating aliasing pointers.
Precisely, so why are we discussing this?
I claim that const is useful. You claim that it can't solve all the worlds
problems. I agree with that, but I maintain it is still useful.
But, in order for it to be useful, it requires that people do not circumvent it
in the wrong places (such as kfree).
--
/ jakob
[email protected] wrote:
> Giacomo Catenazzi wrote:
>
>> const No writes through this lvalue. In the absence of this qualifier, writes may occur
>> through this lvalue.
>>
>> volatile No cacheing through this lvalue: each operation in the abstract semantics must
>> be performed (that is, no cacheing assumptions may be made, since the location
>> is not guaranteed to contain any previous value). In the absence of this qualifier,
>> the contents of the designated location may be assumed to be unchanged except
>> for possible aliasing.
>
> Well, I'm still wondering if there is not something dangerous or weird about
> declaring the argument of kfree() to be const...
It should be only cosmetic thing (and few warnings in some
not yet identified cases).
> Since the content of the referenced object is unlikely to be declared volatile, the
> compiler should be allowed to assume that its content was not changed, except
> for possible aliasing. But what happens if the compiler can also prove there is
> no aliasing? In that case, he should be allowed to assume that the content
> pointed to was not modified at all, right?
I doesn't follow it. Anyway C has the "as-if" rule, which it mean:
the compiler could optimize as far the result doesn't change
(time are not issues, and result could change if a valid compiler
could give the same results). So a very smart compiler (which
should compile all units at the same time) could do good things
without need of explicit register, static (with some exceptions),
const, volatile (if it very smart it know about system, signals,
and it can set "volatile" on need), restrict, ...
> Fortunately, kmalloc is not declared with attribute malloc in the kernel,
> so there should be no problem, but if it were (and, actually, I've not found
> why it wasn't), the compiler would be able to tell that *s1 *cannot*
> be aliased, and therefore decide to move val = s1->i *after* having
> called kfree(). In that case, we would clearly have a bug...
IIRC kmalloc(0) return an alias (but not so relevant in this
discussion).
Hmm. C is used not to do much optimization. One thing to remember
is that function could have a lot of side effects, so compiler
will/should never optimize in your way (but if compiler know exactly
how kfree work internally).
C "const" is a lot weaker to C++ "const".
BTW, I doesn't like const in kfree, but I was talking about
weak "const" in C.
> So, although this should currently work, code which breaks if you do
> a legitimate modification somewere else looks quite dangerous to me.
for sure! But C is anal in: "users know better than compiler on what
they want to do", so compiler cannot do big optimizations C, without
breaking C rules, and used can do nasty things playing with hidden
pointers.
ciao
cate
PS: use lkml rule: "do CC: to all relevant people!"
David Schwartz wrote:
> 2) The 'kfree' operation changes the logical state of the object pointed to,
> as the object goes from existent to non-existent.
I don't think that kfree() itself changes the state of the object. It
doesn't call a destructor or anything like that, so the object itself
must be "inert" before the call to kfree(). That is, at the time of the
kfree() call the system must have ensured that the object will no longer
be used by anything.
The call to kfree() is simply bookkeeping--allowing that memory to be
reused by other parts of the kernel.
> 3) It is most useful for 'kfree' to be non-const because destroying an
> object through a const pointer can easily be done in error. One of the
> reasons you provide a const pointer is because you need the function you
> pass the pointer to not to modify the object. Since this is an unusual
> operation that could be an error, it is logical to force the person doing it
> to clearly indicate that he knows the pointer is const and that he knows it
> is right anyway.
I have a certain amount of sympathy for this view...it's a fairly
painless way to reduce the likelihood of errors. At the same time, I
don't think I've ever run into this problem myself--is it really all
that common?
Chris
On Thu, 17 Jan 2008, David Schwartz wrote:
>
> Nonsense. The 'kfree' function *destroys* the object pointer to by the
> pointer. How can you describe that as not doing anything to the object?
Here's an idea. Think it through.
Why don't we need write permissions to a file to unlink it?
Here's a hint: because unlinking doesn't *write* to it. In fact, it
doesn't read from it either. It doesn't do any access at all to that
object, it just *removes* it.
Is the file gone after you unlink it? Yes (modulo refcounting for aliasing
"pointers" aka filenames, but that's the same for any memory manager -
malloc/free just doesn't have any, so you could think of it as a
non-hardlinking filesystem).
So you're the one who are speaking nonsense. Making something "not exist"
is not at all the same thing as accessing it for a write (or a read). It
is a metadata operation that doesn't conceptually change the data in any
way, shape or form - it just makes it go away.
And btw, exactly as with kfree(), a unlink() may well do something like
"disk scrubbing" for security purposes, or cancel pending writes to the
backing store. But even though it may write (or, by undoing a pending
write, effectively "change the state") to the disk sectors that used to
contain the file data, ONLY AN IDIOT would call it "writing to the file".
Because "the file" is gone. Writing to the place where the file used to be
is a different thing.
So give it up. You're wrong. Freeing a memory area is not "writing to it"
or accessing it in *any* manner, it's an operation on another level
entirely.
Linus
Giacomo A. Catenazzi wrote :
> [email protected] wrote:
> > Giacomo Catenazzi wrote:
> >
> >> const No writes through this lvalue. In the absence of this qualifier, writes may occur
> >> through this lvalue.
> >>
> >> volatile No cacheing through this lvalue: each operation in the abstract semantics must
> >> be performed (that is, no cacheing assumptions may be made, since the location
> >> is not guaranteed to contain any previous value). In the absence of this qualifier,
> >> the contents of the designated location may be assumed to be unchanged except
> >> for possible aliasing.
> >
> > Well, I'm still wondering if there is not something dangerous or weird about
> > declaring the argument of kfree() to be const...
>
> It should be only cosmetic thing (and few warnings in some
> not yet identified cases).
>
>
> > Since the content of the referenced object is unlikely to be declared volatile, the
> > compiler should be allowed to assume that its content was not changed, except
> > for possible aliasing. But what happens if the compiler can also prove there is
> > no aliasing? In that case, he should be allowed to assume that the content
> > pointed to was not modified at all, right?
>
> I doesn't follow it. Anyway C has the "as-if" rule, which it mean:
> the compiler could optimize as far the result doesn't change
> (time are not issues, and result could change if a valid compiler
> could give the same results).
As long as the code is correct, yes. If the code is wrong in the first
place (like adding or removing a required type qualifier), the compiler
optimization can break your code. I'm arguing that having at the same
time kfree() declared as taking a const pointer and kmalloc() with
attribute malloc would be incorrect.
Let me rephrase my argumentation : if the compiler knows a) that a
variable is never modified through a given lvalue (because this pointer
is declared as const), and b) that it is also never modified through any
other lvalues (because it *knows* that no pointer can point to it, and it
can see all other uses), THEN it knows it is not modified at all.
And it can use this knowledge to do all optimizations it wants.
The malloc attribute is exactly about this : giving the compiler the
indication that no other pointer aliases this object, allowing for
better optimizations.
>
> > Fortunately, kmalloc is not declared with attribute malloc in the kernel,
> > so there should be no problem, but if it were (and, actually, I've not found
> > why it wasn't), the compiler would be able to tell that *s1 *cannot*
> > be aliased, and therefore decide to move val = s1->i *after* having
> > called kfree(). In that case, we would clearly have a bug...
>
> IIRC kmalloc(0) return an alias (but not so relevant in this
> discussion).
>
> Hmm. C is used not to do much optimization. One thing to remember
> is that function could have a lot of side effects, so compiler
> will/should never optimize in your way (but if compiler know exactly
> how kfree work internally).
>
> C "const" is a lot weaker to C++ "const".
>
> BTW, I doesn't like const in kfree, but I was talking about
> weak "const" in C.
>
>
> > So, although this should currently work, code which breaks if you do
> > a legitimate modification somewere else looks quite dangerous to me.
>
> for sure! But C is anal in: "users know better than compiler on what
> they want to do", so compiler cannot do big optimizations C, without
> breaking C rules, and used can do nasty things playing with hidden
> pointers.
Yes. Bad things start to happen when users add wrong indications to
the compiler. By adding the "const" indication to kfree(), the programmer
wrongly tells that it can optimize reading the values pointed to before or
after calling the function (if it is also sure that they cannot be
read/written otherwise). Current gcc implementations seem quite
conservative in this regard, and don't optimize that much, but what about
the future?
Cheers,
Emmanuel Colbus
>
> PS: use lkml rule: "do CC: to all relevant people!"
Sorry, actually, I'm not subscribed myself, I was just
following this discussion through hypermail.
On Fri, Jan 18, 2008 at 08:53:44AM -0500, Andy Lutomirski wrote:
> I'd say this implies the exact opposite. It almost sounds like the
> compiler is free to change:
>
> void foo(const int *x);
> foo(x);
> printf("%d", x);
>
> to:
>
> void foo(const int *x);
> printf("%d", x);
> foo(x);
That's only if neither function has side effects noticeable by the
other. Invalidating the pointer in (k)free is rather noticeable.
> (Note that this isn't just a problem for optimizers -- a programmer
> might expect that passing a pointer to a function that takes a const
> pointer argument does not, in and of itself, change the pointed-to
> value. Given that const certainly does not mean that no one else
> changes the object, I'm not sure what else it could mean.
Most of the time, const pointer arguments means "I won't change the
contents of the object so that you'll notice by reading it in a normal
way afterwards". That's pretty much what mutable in a variety of
languages (including C++) is about, saying "this field is internal
management stuff not visible from the external interface, so I need to
be able to change it even through const pointers I got as parameters".
Reference counters for copy-on-write setups is the usual example of
use.
In the case of deallocation functions you are not allowed to do
anything through the pointer or its aliases after the function
returns. So we're outside of the "most of the time" case, since
you're not allowed to try to notice any change. Pragmatism takes
over, you want the type that catches as many possible types as
possible while staying reasonable (volatile is never reasonable), and
that's const void *. As simple as that.
As for releasing resources through const pointers, that happens all
the time as soon as your const use is tight, and if you think forcing
the systematic addition of a (void *) cast is going to make your code
more readable, well, you need more experience in maintaining other
people's applications.
> kfree does not have either property, so I'm don't think it makes
> sense for it to take a const argument.
delete in C++ allows const pointers. Think about it.
OG.
On Thu, Jan 17, 2008 at 09:02:44PM -0800, David Schwartz wrote:
> 3) It is most useful for 'kfree' to be non-const because destroying an
> object through a const pointer can easily be done in error. One of the
> reasons you provide a const pointer is because you need the function you
> pass the pointer to not to modify the object. Since this is an unusual
> operation that could be an error, it is logical to force the person doing it
> to clearly indicate that he knows the pointer is const and that he knows it
> is right anyway.
Freeing a const pointer is not and has never been unusual. It happens
all the time for objects whose lifecycle is "initialise at the start,
readonly afterwards", of which names, in particular in the form of
strings, are a large subset. It also happens in cases of late
deletion on refcounted objects, when the main owner (the one who is
allowed to change the object and has the non-const pointer) has
dropped its reference, but some object needs a readonly instance a
little longer. Think virtual files in proc, sysfs or friends kept
open after the underlying information source, often a device, is gone.
OG.
On Jan 18, 2008 6:02 AM, David Schwartz <[email protected]> wrote:
>
> However, *destroying* an object is not a metadata operation -- it destroys
> the data as well. This is kind of a philosophical point, but an object does
> not have a "does this object exist" piece of metadata. If an object does not
> exist, it has no data. So destroying an object destroys the data and is thus
> a write/modification operation on the data.
>
In C++ you can delete a const pointer. Now I know kernel hackers
aren't especially impressed with C++ but maybe someone could look up
the rationale for that design decision (I couldn't find it). It might
shed some light on this discussion.
/DM
On Fri, Jan 18, 2008 at 05:45:49PM +0100, [email protected] wrote:
> The malloc attribute is exactly about this : giving the compiler the
> indication that no other pointer aliases this object, allowing for
> better optimizations.
If you put a malloc attribute on the allocator and no free attribute
on the deallocator, you can get bugs indeed. GIGO.
> Yes. Bad things start to happen when users add wrong indications to
> the compiler. By adding the "const" indication to kfree(), the programmer
> wrongly tells that it can optimize reading the values pointed to before or
> after calling the function (if it is also sure that they cannot be
> read/written otherwise). Current gcc implementations seem quite
> conservative in this regard, and don't optimize that much, but what about
> the future?
The future should be quite nice because:
- the compiler can not know that kmalloc does not have an alias to
the pointer tucked somewhere accessible by other non-inline functions
(as kfree is), especially since it does have aliases in practice, so
it cannot prove to "not read/written otherwise" part without the
malloc attribute
- if you add the (non-standard C) malloc attribute to kmalloc, then
you also add the free attribute to kfree which tells the compiler
that the pointer is invalid after the call, which ensures no
accesses will be moved after it
OG.
On Friday 18 January 2008 03:47:01 am Giacomo A. Catenazzi wrote:
> Changing the name of variables in your example:
>
> extern print_int(const int *);
>
> int main(int argc, char **argv)
> {
> extern int errno;
>
> errno = 0;
> print_int(&i);
> return errno;
> }
Except that changing int to extern int makes all the difference in the world:
the variable went from being local to being global. The way const is
currently defined, however, the compiler cannot take advantage of the fact
that the variable was local in the former case.
> Ok, I changed int to extern int, but you see the point?
> Do you want complex rules about const, depending on
> context (extern, volatile,...) ?
Sometimes complexity is worth it.
-- Vadim Lobanov
Olivier Galibert wrote:
> On Fri, Jan 18, 2008 at 05:45:49PM +0100, ecolbus@xxxxxxxx wrote:
> > The malloc attribute is exactly about this : giving the compiler the
> > indication that no other pointer aliases this object, allowing for
> > better optimizations.
>
> If you put a malloc attribute on the allocator and no free attribute
> on the deallocator, you can get bugs indeed. GIGO.
>
>
> > Yes. Bad things start to happen when users add wrong indications to
> > the compiler. By adding the "const" indication to kfree(), the programmer
> > wrongly tells that it can optimize reading the values pointed to before or
> > after calling the function (if it is also sure that they cannot be
> > read/written otherwise). Current gcc implementations seem quite
> > conservative in this regard, and don't optimize that much, but what about
> > the future?
>
> The future should be quite nice because:
>
> - the compiler can not know that kmalloc does not have an alias to
> the pointer tucked somewhere accessible by other non-inline functions
> (as kfree is), especially since it does have aliases in practice, so
> it cannot prove to "not read/written otherwise" part without the
> malloc attribute
>
> - if you add the (non-standard C) malloc attribute to kmalloc, then
> you also add the free attribute to kfree which tells the compiler
> that the pointer is invalid after the call, which ensures no
> accesses will be moved after it
>
> OG.
I completely agree with you here. But, unless this page :
http://gcc.gnu.org/onlinedocs/gcc/Function-Attributes.html
is not up-to-date anymore, there is no free attribute. Which
is quite logical, actually, as such an attribute would be
equivalent to simply marking the parameter of the function
non-const.
That's what I was alluding to, when speaking of a legitimate
change (adding attribute malloc to kmalloc()) which would break
another part of the kernel. Of course, we can also assume that
nobody would make the error of letting kfree() taking a
const void* after that, but experience shows that programmers
are very likely to fall into such traps.
(No flame intended on anybody : in my case, I'm actually able
to fall even when there is no trap at all. That's the reason for
spotting such a weird one makes me anxious. Especially since
having kfree() take a non-const argument comes at no runtime
cost.)
Cheers,
Emmanuel
On Friday 18 January 2008 05:54:12 am Andy Lutomirski wrote:
> It almost sounds like the compiler is free to change:
>
> void foo(const int *x);
> foo(x);
> printf("%d", x);
>
> to:
>
> void foo(const int *x);
> printf("%d", x);
> foo(x);
>
> especially if it can prove that the pointer to x doesn't otherwise
> escape or that foo doesn't call anything that could see the pointer (and
> given that gcc has special magical markings for malloc, one way this
> could be "proven" is to have x be some freshly malloced object.
That's absolutely not true.
Let's unravel the code, by fixing usage of 'x' (which seems to vary at will
between value and pointer in the above example), and by replacing printf with
another opaque function. Our decls:
void foo(const int *ptr);
void bar(int val);
You're saying that this:
foo(&x);
bar(x);
can be reordered into this:
bar(x);
foo(&x);
No way. First, the way that const is currently defined, the compiler cannot
assume that the value of x did not change while foo was executing. So, it
will not only be forced to leave the two functions in that order, it will
even reload the value of x before passing it into bar. Go figure.
Second, even if const did have stronger semantics that forbade the value of x
from being modified during execution of foo, the compiler still could not
reorder the two function calls, before it cannot assume that the two
functions (in their internal implementations) do not touch some other,
unknown to this code, global variable.
-- Vadim Lobanov
On Fri, 2008-01-18 at 11:14 -0800, Vadim Lobanov wrote:
[cut]
> Second, even if const did have stronger semantics that forbade the value of x
> from being modified during execution of foo, the compiler still could not
> reorder the two function calls, before it cannot assume that the two
> functions (in their internal implementations) do not touch some other,
> unknown to this code, global variable.
This is why GCC has the pure and const function attributes. These
attributes are more powerful than the "const" keyword, and do allow
optimizations with assumptions about global state.
--
Zan Lynx <[email protected]>
On Friday 18 January 2008 11:31:05 am Zan Lynx wrote:
> On Fri, 2008-01-18 at 11:14 -0800, Vadim Lobanov wrote:
> > Second, even if const did have stronger semantics that forbade the value
> > of x from being modified during execution of foo, the compiler still
> > could not reorder the two function calls, before it cannot assume that
> > the two functions (in their internal implementations) do not touch some
> > other, unknown to this code, global variable.
>
> This is why GCC has the pure and const function attributes. These
> attributes are more powerful than the "const" keyword, and do allow
> optimizations with assumptions about global state.
Oh, absolutely. The problem, however, is that very very few people actually
use these function attributes in their code. Heck, we don't even use the
standard restrict keyword, much less gcc-specific function annotations.
I think that one part of the problem is because gcc seems to have had
attribute diarrhea -- I know of noone who can recite more than 10% of the
available attributes without glancing at the documentation. The other part of
the problem is that gcc does no sanity checks on the provided attributes. For
example, the code below compiles perfectly fine without a peep, even
with -Wall and -Wextra.
int global;
void foobar(const int *x) __attribute__((const));
void foobar(const int *x)
{
if (*x)
*(int *)x = 7;
global = 11;
}
*grumble, grumble* :-)
-- Vadim Lobanov
> On Thu, 17 Jan 2008, David Schwartz wrote:
> > Nonsense. The 'kfree' function *destroys* the object pointer to by the
> > pointer. How can you describe that as not doing anything to the object?
>
> Here's an idea. Think it through.
>
> Why don't we need write permissions to a file to unlink it?
You cannot unlink a file. Given a file, if you were to attempt to unlink it,
what directory would you remove it from?
Unlinking a file is an operation on the directory the file is in, not on the
directory itself. We do need write permissions to the directory.
If you had only a const pointer to the data in the file, you should
definitely not be able to use that pointer to find a directory the file is
in and remove it without clearly indicating you know *exactly* what you're
doing. Given just that 'const' pointer, you're not supposed to be modifying
the data and certainly using that pointer to modify anything logically above
it.
> Here's a hint: because unlinking doesn't *write* to it. In fact, it
> doesn't read from it either. It doesn't do any access at all to that
> object, it just *removes* it.
Right. It's an operation on the directory the file is in that might have
consequences for the file.
> Is the file gone after you unlink it? Yes (modulo refcounting for
> aliasing
> "pointers" aka filenames, but that's the same for any memory manager -
> malloc/free just doesn't have any, so you could think of it as a
> non-hardlinking filesystem).
The file is gone if and only if the directory was the only thing that needed
the file to exist. A file that is only on one directory "belongs to" that
directory. So write permission to the directory is all that is needed.
What you are arguing is essentially that you should be able to remove a file
from any directory it is in just because you have write access to the file's
data.
> So you're the one who are speaking nonsense. Making something "not exist"
> is not at all the same thing as accessing it for a write (or a read). It
> is a metadata operation that doesn't conceptually change the data in any
> way, shape or form - it just makes it go away.
Making something "not exist" is a modification operation on that thing.
> And btw, exactly as with kfree(), a unlink() may well do something like
> "disk scrubbing" for security purposes, or cancel pending writes to the
> backing store. But even though it may write (or, by undoing a pending
> write, effectively "change the state") to the disk sectors that used to
> contain the file data, ONLY AN IDIOT would call it "writing to the file".
> Because "the file" is gone. Writing to the place where the file
> used to be
> is a different thing.
I agree with you about that part. I can't understand why you keep thinking
this is where our disagreement lies when I've stated at least three times
that I agree about this. The issue has nothing to do with whether or not
'kfree' modifies the particular bytes pointed to. It has to do with whether
or not 'kfree' is the kind of operation one would normally want to allow on
a 'const' object.
> So give it up. You're wrong. Freeing a memory area is not "writing to it"
> or accessing it in *any* manner, it's an operation on another level
> entirely.
Nevertheless, it's a modification operation on an object that's not supposed
to be modified.
By the way, I did think of one argument that supports your position: Suppose
you have a reference counted object. You have a 'release reference and free
if zero' function. Should it be 'const'? If not, how can a 'lookup and
reference for read' function return a const pointer to the object?
However, on balance, I think a 'release reference and free if zero' function
that operates on a const pointer is sufficiently unusual that a cast to show
you know what you're doing is not a bad thing. In this case, you know it's
safe to destroy the object through a const pointer because you *know* nobody
gave you the 'const' pointer trusting you not to destroy the object. A cast
to show you have that special knowledge is, IMO, reasonable.
DS
On Fri, 18 Jan 2008 18:24:29 +0100, Olivier Galibert <[email protected]> wrote:
> On Fri, Jan 18, 2008 at 08:53:44AM -0500, Andy Lutomirski wrote:
> > I'd say this implies the exact opposite. It almost sounds like the
> > compiler is free to change:
> >
> > void foo(const int *x);
> > foo(x);
> > printf("%d", x);
> >
> > to:
> >
> > void foo(const int *x);
> > printf("%d", x);
> > foo(x);
>
> That's only if neither function has side effects noticeable by the
> other. Invalidating the pointer in (k)free is rather noticeable.
>
That's what __attribute__ ((pure)) is for, but if none of the
functions is pure, the compiler can not be sure about side effects
and can not reorder things. Don't forget that functions can do
anything apart from mangling with their arguments.
And allocator/deallocator functions never can be pure, they must
change global data, the pool of free blocks.
--
J.A. Magallon <jamagallon()ono!com> \ Software is like sex:
\ It's better when it's free
Mandriva Linux release 2008.1 (Cooker) for i586
Linux 2.6.23-jam05 (gcc 4.2.2 20071128 (4.2.2-2mdv2008.1)) SMP PREEMPT
"J.A. Magall?n" <[email protected]> writes:
> That's what __attribute__ ((pure)) is for, but if none of the
> functions is pure, the compiler can not be sure about side effects
> and can not reorder things. Don't forget that functions can do
> anything apart from mangling with their arguments.
Though it seems it could legally transform:
void kfree(const int *x);
{
int v, *ptr = malloc(sizeof(int));
*ptr = 51;
v = *ptr;
kfree(ptr);
printf("%d", v);
into:
{
int v, *ptr = malloc(sizeof(int));
*ptr = 51;
kfree(ptr);
v = *ptr;
printf("%d", v);
}
if it knows that malloc generates unaliased pointers, which seems
reasonable in general.
--
Krzysztof Halasa