Help with pointer type casting

[Eclipse07]

Hi all,

I came across some code and it's not clear why it is casting an unsigned char * to another pointer type only to free it right after.
Here are the relevant structures:

struct _Edje_Message
{
   Edje              *edje;
   Edje_Queue         queue;
   Edje_Message_Type  type;
   int                id;
   unsigned char     *msg;
   Eina_Bool          propagated : 1;
};

struct _Edje_Message_String
{
   char *str; /* The message's string pointer */
};

struct _Edje_Message_Int
{
   int val; /* The message's value */
};

As you can see, _Edge_Message has a *msg field, but in the function below, they cast it to the other two structure types inside the case blocks of the switch statement only to free it. What is the point or advantage of doing this?

void
_edje_message_free(Edje_Message *em)
{
   if (em->msg)
     {
	int i;

	switch (em->type)
	  {
	   case EDJE_MESSAGE_STRING:
	       {
		  Edje_Message_String *emsg;

		  emsg = (Edje_Message_String *)em->msg;
		  free(emsg->str);
		  free(emsg);
	       }
	     break;
	   case EDJE_MESSAGE_INT:
	       {
		  Edje_Message_Int *emsg;

		  emsg = (Edje_Message_Int *)em->msg;
		  free(emsg);
	       }
	     break;

[c99tutorial]

It looks like the purpose is to determine the type and then cast the void pointer to the appropriate type. Normally you make a void* to do this, but some programmers use unsigned char * for this purpose, as was done here. Either way, this code looks dubious to me because I don't see any guarantee that the pointer will be properly aligned for an int as well as a char.

[Eclipse07]

I see. Thanks for the input. But even if they cast it to another type before freeing it, if free will accept it as a void * anyway, how would alignment or anything else matter?

[Hodor]

I see. Thanks for the input. But even if they cast it to another type before freeing it, if free will accept it as a void * anyway, how would alignment or anything else matter?

I don't think alignment would matter at all; it's a pointer type and, well, I can't see how alignment of the pointer type would change by casting it to a pointer to a different type

[tabstop]

I see. Thanks for the input. But even if they cast it to another type before freeing it, if free will accept it as a void * anyway, how would alignment or anything else matter?

The alignment isn't changing or anything. But compilers warn about assigning pointers to the wrong type, and an explicit cast will silence those warnings.

[anduril462]

@Eclipse07:
Notice that in the EDJE_MESSAGE_STRING case, they free(emsg->str) and in the EDJE_MESSAGE_INT they don't? They convert the generic pointer em->msg to a specific type emsg. They then take that type and free any of it's members, if appropriate. That cast just keeps the compiler from complaining about converting from unsigned char * to Edje_Message_String * or Edje_Message_Int *. Then, once they cast the generic pointer to the right type, they can access any members they need to free, like emsg->str in the case of Edje_Message_String.

Where alignment becomes an issue in type casting is if you cast between two things that might not have the same alignment, then attempt to access that memory via the pointer:

short s[2];  // assume shorts are 2 bytes each, required to be aligned on addresses that are multiples of 2 (e.g. 0x1002)
int *ip = s;  // assume 4-byte int, required to be aligned on addresses that are multiples of 4 (e.g. 0x1000 or 0x1004)
*ip = 42;  // try to write an int to the 4 bytes in s might generate an error

Note, I say "might" generate an error. Some systems may not have such alignment restrictions, or some compilers may generate extra instructions to work around the alignment issue (usually with a speed cost). Point is, it's dangerous, so avoid it.

@c99tutorial:
The posted code is guaranteed to have proper alignment for the int and char members. The memory was allocated with malloc/calloc/realloc (since they're freeing it) which means it must be suitably aligned for any object type:

1 The order and contiguity of storage allocated by successive calls to the calloc,
malloc, and realloc functions is unspecified. The pointer returned if the allocation
succeeds is suitably aligned so that it may be assigned to a pointer to any type of object
and then used to access such an object or an array of such objects in the space allocated
(until the space is explicitly deallocated). The lifetime of an allocated object extends
from the allocation until the deallocation. Each such allocation shall yield a pointer to an
object disjoint from any other object. The pointer returned points to the start (lowest byte
address) of the allocated space. If the space cannot be allocated, a null pointer is
returned. If the size of the space requested is zero, the behavior is implementation-
defined: either a null pointer is returned, or the behavior is as if the size were some
nonzero value, except that the returned pointer shall not be used to access an object.

The standard guarantees that the first member of a struct will be at the same address as the struct itself (I feel like it's stated more explicitly somewhere else, but can't find that section right now):

13 Within a structure object, the non-bit-field members and the units in which bit-fields
reside have addresses that increase in the order in which they are declared. A pointer to a
structure object, suitably converted, points to its initial member (or if that member is a
bit-field, then to the unit in which it resides), and vice versa. There may be unnamed
padding within a structure object, but not at its beginning.

So emsg->str and emsg->val will be aligned the same as emsg, which is really just em->msg, which is apparently returned by an alloc function, thus they should have the same alignment. As a side note, depending on your reading of the standard, it doesn't guarantee that two types, e.g. struct foo and struct bar have the same alignment requirements, and thus casting between the two may be dangerous if the objects are not in memory returned by an alloc function or otherwise guaranteed to have compatible alignment. In practice, I'm not sure this ever happens.

Also, it is possible that the type member was set incorrectly, in which case free(emsg->str) might actually be trying to free an int value (emsg->val) that was never actually returned from an alloc function, but that's a whole different problem.

@Hodor:
It's not that the cast itself changes the alignment. Casting effectively tells the compiler "I know more than you, don't worry or warn me about alignment". It allows you to make a pointer of a certain type point to an object of a different type which may be misaligned for the actual pointed-to type. Usually you want to avoid this since different pointer types may have different alignment requirements, and casting will hide any such errors at compile time and they may then blow up at run time.

[Hodor]

[...]

You're probably right. I just had a quick look at the standard and it's kind of complicated. My original thoughts were along the lines that C didn't originally have void (and therefore no void *) and char * was used as a catch-all/generic pointer type and that somehow possessed me make the startling conclusion that casting a char * to another pointer type (and vice-versa) was somehow special and not undefined behaviour.

Perhaps the original code is very outdated. Why not change unsigned char *msg; to void *msg; and be done with it?

[laserlight]

My original thoughts were along the lines that C didn't originally have void (and therefore no void *) and char * was used as a catch-all/generic pointer type and that somehow possessed me make the startling conclusion that casting a char * to another pointer type (and vice-versa) was somehow special and not undefined behaviour.

The vice-versa situation is well defined (for pointers to objects).

[anduril462]

You're probably right. I just had a quick look at the standard and it's kind of complicated. My original thoughts were along the lines that C didn't originally have void (and therefore no void *) and char * was used as a catch-all/generic pointer type and that somehow possessed me make the startling conclusion that casting a char * to another pointer type (and vice-versa) was somehow special and not undefined behaviour.

That's the gist of it. The alignment of void * and char * is the same (see C99 6.2.5 p26). Alignment issues really only surface when you dereference a pointer (i.e. access the data pointed to). The C99 Rationale (I.e. "why C is the way it is") discusses that in a bit more detail in section 6.3.2.3 (p48 in my copy).

Perhaps the original code is very outdated. Why not change unsigned char *msg; to void *msg; and be done with it?

Outdated, or written by somebody with very outdated knowledge of C. That is certainly the fix I would make, but keep in mind that the real benefit of that change is removing all the casts, and there may be a whole lot of them.

[Hodor]

..., but keep in mind that the real benefit of that change is removing all the casts, and there may be a whole lot of them.

Correct :-)

[Eclipse07]

Thanks anduril462, you seem really knowledgeable. I guess the reason I started this thread was because, since I'm learning that when you cast an address to be a specific pointer type, that structure "map" sort of "overlays" the original memory area and when you access members they start at that address i.e. emsg->str is actually the original *msg (if i'm understanding this correctly). It just struck me as odd that they do all this casting and why not just do free(em->msg); to begin with? This code actually comes from the Enlightenment 0.17 window manager that just recently came out (enlightenment.org). All I know is the developers are probably very good at C and the project has a codebase exceeding 1 million lines of code.

[tabstop]

It just struck me as odd that they do all this casting and why not just do free(em->msg); to begin with?

That was the point anduril was making -- em->msg has more fields and so those fields have to be freed as well. So they have to free em->msg->str before they can free em->msg.

[anduril462]

Thanks anduril462, you seem really knowledgeable.

Sometimes I appear knowledgeable, other times I look like the worlds biggest dumbass. I guess today is it's more the former.

I guess the reason I started this thread was because, since I'm learning that when you cast an address to be a specific pointer type, that structure "map" sort of "overlays" the original memory area and when you access members they start at that address i.e. emsg->str is actually the original *msg (if i'm understanding this correctly).

More or less, yes, that's it. It's a way to add some OO-like features such as inheritance and polymorphism to C (which is not OO).

This code actually comes from the Enlightenment 0.17 window manager that just recently came out (enlightenment.org). All I know is the developers are probably very good at C and the project has a codebase exceeding 1 million lines of code.

That's only a somewhat reasonable assumption. Size of a code base, or the fact that it's open source have little bearing on quality, though I would suspect that the Enlightenment code base is pretty decent overall (still actively developed and used for 15+ years). One problem with open-source projects is that there are a lot of developers, and depending on how good the people who run the project are, and how thoroughly they review the code being submitted by others, they may let in lots of sub-par code. The "unnecessary" casting may also be a result of them trying to be as accommodating/portable as possible, in case somebody wants to build this on a system that for some ungodly reason doesn't even support the 24-year-old C89 standard.

As a side note, I seem to recall E17 being in development back in the early 2000s. I sure as hell hope it's good code after a decade of working on it.

[Eclipse07]

True about the quality thing. I think I do understand the casting part, and how emsg is effectively *msg itself address-wise (but cast to another structure type with members). So if that is in fact true, then the first member (emsg->str) is effectively *msg too (the beginning of the structure). As shown in the code it appears they free the same address twice. I don't know, maybe a memory diagram would help but this pointer stuff is ridiculous - i'm not sure anymore I want to do this for a living.

[tabstop]

True about the quality thing. I think I do understand the casting part, and how emsg is effectively *msg itself address-wise (but cast to another structure type with members). So if that is in fact true, then the first member (emsg->str) is effectively *msg too (the beginning of the structure). As shown in the code it appears they free the same address twice. I don't know, maybe a memory diagram would help but this pointer stuff is ridiculous - i'm not sure anymore I want to do this for a living.

It's located at the beginning of the structure, that is true; but it's location is irrelevant. The memory is being pointed to. emsg->str might live at memory address 97, but if it contains (as a value, and therefore points to) memory address 143, it's memory address 143 that gets freed, not 97.