11 replies to this topic

[ultramailman]

Hello, this bothered me for a long time...
How do I correctly free something that is pointed to by a pointer to const?

Look at this:

struct pair{
	 void const * key;
	 void const * val;
};

Here I have a struct that is to be used for hash tables. I make them const because the keys are not supposed to change while they are inside a hash table. If the key is changed, that would mess up the table. This helped a lot, compilation would fail if I accidentaly modify the key. This is all good until I am finished with the pair and decide to free the key and val:

void free(void * ptr);

If I try to free key, compilation fails, because the prototype of free makes no guarantees of not modifying whatever is passed in, like this:

void free(void const * ptr);

Right now, I handle this by casting the pointer to non const, then free it:

void FREE(void const * ptr)
{
	 free((void *)ptr);
}

This lets me compile, but it looks evil.

After searching around, I found kfree:

void kfree(const void *objp);

But I can't use that, because it is part of linux, and not of the C standard.

Am I just being paranoid, and casting to non const is fine? Or is there another solution? Assuming struct pair is opaque, and key and val are always malloced and memcpy of argument pointers.

[Ashaman73]

The only really clean way is to rewrite your logic. Everything else is a hack.
E.g. instead of using a struct pair, use a class and add some const accessor and some friend none-const manipulator for the class, which should be able to clean it up.

friend class HashMap;
class pair {
private:
void *key;
void *val;
protected:
  void* getKey()...
   void setKey(void* ...)
public:
   const void getKey();
   const void getValue();
}

[ultramailman]

The only really clean way is to rewrite your logic. Everything else is a hack.
E.g. instead of using a struct pair, use a class and add some const accessor and some friend none-const manipulator for the class, which should be able to clean it up.

friend class HashMap;
class pair {
private:
void *key;
void *val;
protected:
  void* getKey()...
   void setKey(void* ...)
public:
   const void getKey();
   const void getValue();
}

Sorry friend, but I can't use classes in C

[BitMaster]

E.g. instead of using a struct pair, use a class and add some const accessor and some friend none-const manipulator for the class, which should be able to clean it up.

Although not actually incorrect, I object to that because the choice of words reinforcess the common fallacy that struct and class are something different in C++. They are exactly the same and completely interchangeably, except for the fact that structs are public by default (for both accessibility and inheritance) and classes are private by default.

That said, this argument only pertains to C++ anyway. I'm not very fluent in C, but shouldn't something like this be some kind of solution?

// header
struct pair;
const void* getKey(struct pair*);
const void* getValue(struct pair*);

// implementation
struct pair
{
   void* key;
   void* val;
};

const void* getKey(struct pair* p)
{
   return p->key;
}

const void* getKey(struct pair* p)
{
   return p->val;
}

Then you need some kind of similar allocator/free functions and some mutators and you have transferred the C++ way of doing things into C. Kinda. In a way. Well, not really but it's the best I can think of right now. Unless someone who knows his wayaround around actual C comes around it might be your best option.

[ultramailman]

E.g. instead of using a struct pair, use a class and add some const accessor and some friend none-const manipulator for the class, which should be able to clean it up.

Although not actually incorrect, I object to that because the choice of words reinforcess the common fallacy that struct and class are something different in C++. They are exactly the same and completely interchangeably, except for the fact that structs are public by default (for both accessibility and inheritance) and classes are private by default.

That said, this argument only pertains to C++ anyway. I'm not very fluent in C, but shouldn't something like this be some kind of solution?

// header
struct pair;
const void* getKey(struct pair*);
const void* getValue(struct pair*);

// implementation
struct pair
{
   void* key;
   void* val;
};

const void* getKey(struct pair* p)
{
   return p->key;
}

const void* getKey(struct pair* p)
{
   return p->val;
}

Then you need some kind of similar allocator/free functions and some mutators and you have transferred the C++ way of doing things into C. Kinda. In a way. Well, not really but it's the best I can think of right now. Unless someone who knows his wayaround around actual C comes around it might be your best option.

... I've never thought of it this way. I guess I got too used to directly accessing fields of non opaque structs. Sometimes the most obvious solutions are right there, but I just can't see it. Thanks, this works nicely.

But if anyone has something else, please post ;o

[Lauris Kaplinski]

The semantics of your key and value is that they are mutable, just not in arbitrary ways. You can free them but not modify them otherwise.
Thus the "evil" part is declaring them "void const *" - not casting them to "void *" in free().

But usually you do not worry about such things in C. It is not perfectly type-safe language and was never intended to be. Thus whatever works and is closest to the actual semantics (without inventing too many hacks) is usually the best.

[wqking]

The type of your key, const void *, implicitly means that you should not free it. It points to a block of memory which is not writable, while "free" will destroy that memory, they are conflicting with each other.

You know you can free the key because you designed the whole program, but from the theory, you can not change the content pointed by "const void *", because it may point to a block memory in the ROM, or whatever.

OK, after talk about "const", let's back to your question. I think you have three methods to do,

1, Keep evil. free((void *)key). As you said, it's evil.

2, Change your logic. A key in a table is just a key, it's used to index the table, it should not own the memory ownership. So beside the key, you should keep a "void *" pointer some elsewhere, and free on that pointer instead of key. However, you may not want to use this method if you allocate a new memory for each key, that will end up with double amount of pointers.

3, OK you are using C not C++, but it doesn't prevent you from thinking in OOP way.
Change "key" to void *, but never use it directly. Instead write a getter function,
const void * getTableKey(MyTable).
And use the getter to access it.

Edited by wqking, 17 November 2012 - 02:29 AM.

[ultramailman]

The semantics of your key and value is that they are mutable, just not in arbitrary ways. You can free them but not modify them otherwise.
Thus the "evil" part is declaring them "void const *" - not casting them to "void *" in free().

But usually you do not worry about such things in C. It is not perfectly type-safe language and was never intended to be. Thus whatever works and is closest to the actual semantics (without inventing too many hacks) is usually the best.

Ah, I see, you are right that C is not very type safe. I guess the best thing to do is to write error-free code as much as possible.

The type of your key, const void *, implicitly means that you should not free it. It points to a block of memory which is not writable, while "free" will destroy that memory, they are conflicting with each other.

You know you can free the key because you designed the whole program, but from the theory, you can not change the content pointed by "const void *", because it may point to a block memory in the ROM, or whatever.

I am not sure on the "not writable" and "read only" part. I think const is just used to tell the compiler "Hey compiler, I don't want this value modified by my code. If you see my code modify this value, treat that as an error!". So it's more like a compile-time error-checking tool to allow the programmer catch his/her own logic mistakes. Well, that's only my guess, I haven't bothered to verify it yet.

Change your logic. A key in a table is just a key, it's used to index the table, it should not own the memory ownership. So beside the key, you should keep a "void *" pointer some elsewhere, and free on that pointer instead of key. However, you may not want to use this method if you allocate a new memory for each key, that will end up with double amount of pointers.

Yeah, I make copies of the key. I think oop is the better way.

[ultramailman]

The semantics of your key and value is that they are mutable, just not in arbitrary ways. You can free them but not modify them otherwise.
Thus the "evil" part is declaring them "void const *" - not casting them to "void *" in free().

But usually you do not worry about such things in C. It is not perfectly type-safe language and was never intended to be. Thus whatever works and is closest to the actual semantics (without inventing too many hacks) is usually the best.

Ah, I see, you are right that C is not very type safe. I guess the best thing to do is to write error-free code as much as possible.

The type of your key, const void *, implicitly means that you should not free it. It points to a block of memory which is not writable, while "free" will destroy that memory, they are conflicting with each other.

You know you can free the key because you designed the whole program, but from the theory, you can not change the content pointed by "const void *", because it may point to a block memory in the ROM, or whatever.

I am not sure on the "not writable" and "read only" part. I think const is just used to tell the compiler "Hey compiler, I don't want this value modified by my code. If you see my code modify this value, treat that as an error!". So it's more like a compile-time error-checking tool to allow the programmer catch his/her own logic mistakes. Well, that's only my guess, I haven't bothered to verify it yet.

Change your logic. A key in a table is just a key, it's used to index the table, it should not own the memory ownership. So beside the key, you should keep a "void *" pointer some elsewhere, and free on that pointer instead of key. However, you may not want to use this method if you allocate a new memory for each key, that will end up with double amount of pointers.

Yeah, I make copies of the key. I think oop is the better way.

[wqking]

I am not sure on the "not writable" and "read only" part. I think const is just used to tell the compiler "Hey compiler, I don't want this value modified by my code. If you see my code modify this value, treat that as an error!". So it's more like a compile-time error-checking tool to allow the programmer catch his/her own logic mistakes. Well, that's only my guess, I haven't bothered to verify it yet.

I'm not quite sure about the history, but don't forget that C was invented to write OS and system software.
So it's possible that a pointer in C is pointing to a block of memory in the ROM, which is really read only.
Of course you are correct that "const" is used for compile time check, what I want to say is that a const pointer may really point to something that's not writable.

[kd7tck]

Don't you have to recast to non const inorder to free.

From what I remember in C, you only free what you allocate with calloc/malloc. Did you allocate memory? If not then just set the pointer to this struct to zero and be done with it. If you made these pointers global then this will not work because you are using singletons, and is considered bad practice. Remember globals are not on the stack they are defined, so make certain no to make const pointers global unless you intend to use it through the life of the entire program. If you did allocate memory then recast to non const and then free.

Personally I would rethink the logic and come up with a system of internal checks and ballances that prevent modification of pointers without using const.

[ultramailman]

I am not sure on the "not writable" and "read only" part. I think const is just used to tell the compiler "Hey compiler, I don't want this value modified by my code. If you see my code modify this value, treat that as an error!". So it's more like a compile-time error-checking tool to allow the programmer catch his/her own logic mistakes. Well, that's only my guess, I haven't bothered to verify it yet.

I'm not quite sure about the history, but don't forget that C was invented to write OS and system software.
So it's possible that a pointer in C is pointing to a block of memory in the ROM, which is really read only.
Of course you are correct that "const" is used for compile time check, what I want to say is that a const pointer may really point to something that's not writable.

I see. When you say ROM, what is the degree of the ROM-ness? Is it some region of ROM on the RAM or is it some "harder" kind of ROMs like the EEPROM in which the BIOS reside in? My question probably sounds stupid, but I am not very knowledgeable on this ;o

Don't you have to recast to non const inorder to free.

From what I remember in C, you only free what you allocate with calloc/malloc. Did you allocate memory? If not then just set the pointer to this struct to zero and be done with it. If you made these pointers global then this will not work because you are using singletons, and is considered bad practice. Remember globals are not on the stack they are defined, so make certain no to make const pointers global unless you intend to use it through the life of the entire program. If you did allocate memory then recast to non const and then free.

Personally I would rethink the logic and come up with a system of internal checks and ballances that prevent modification of pointers without using const.

Oh yes, they are all malloced. I make sure to do a copy of whatever is passed in, so key and val will point to malloced memory. That way I won't accidently free them while the table still has a pointer to them. And no globals are involved here, although I guess anything that is malloced is global, in a way.