void ** and casting (from Writing Solid Code)

I picked up a second-hand copy of Steve Maguire's Writing Solid Code the other day for a few pennies, and had a quick read. In the third chapter, he advocates surrounding complex systems with "gate keeper" functions that are much safer. As an example, he uses C's malloc/free/realloc functions.

Essentially, he advocates this: (compacted for brevity)

Code:

bool alloc(void **p, size_t z)
{
   byte *r = malloc(z);
   if (r == NULL) return false;
   *p = r;
   return true;
}

And then calling it like this:

Code:

char *blk;

if (alloc(&blk, 128) == true) {
  /* do something with blk */
} else {
  /* handle error condition */
}

He advocates this style because it does not combine two different types of result (in malloc's case, pointer and error condition) into one return value. I kinda buy that.

But the problem I spotted immediately is that one cannot cast the address of a char pointer to a void **, without generating a compiler warning (or worse) relating to implicit casting of pointers. The only way I can see to make this work is to wrap the function call in a macro that uses (void **), but that destroys any type safety (you can then pass anything in!)

So, what am I missing? A quick google suggests nobody else has spoken about these issues from this book.

The idea is completely broken on any machine that has different pointer representations for different types (and yes, there have been several. Just think of archaic DOS and all the fun people had with near and far).
Question 5.17

The comfortable C magic of converting void* into another pointer type only works when the compiler can see the type of both pointers at the same time.
Eg in
char *p = malloc(10); // malloc returns void *

The big problem with the other approach is that the compiler can only see void* in the function. Say for example char* were 4 bytes, and void* were 8 bytes. Inside that function, it would take *p = r; and assume (it has no other info) that *p is a pointer to 8 bytes of memory. So the assignment merrily scribbles on 8 bytes, and the 4 bytes past the end of your char* pointer variable are gone.

The straight-forward assignment of malloc doesn't have this problem. The compiler sees 8 bytes returned in void* and sees a 4-byte char* and knows what to do to make it all work out.

Your compiler is correct, it is warning you about something which is dangerous.

Originally Posted by Salem

The idea is completely broken on any machine that has different pointer representations for different types (and yes, there have been several. Just think of archaic DOS and all the fun people had with near and far).
Question 5.17

The comfortable C magic of converting void* into another pointer type only works when the compiler can see the type of both pointers at the same time.
Eg in
char *p = malloc(10); // malloc returns void *

The big problem with the other approach is that the compiler can only see void* in the function. Say for example char* were 4 bytes, and void* were 8 bytes. Inside that function, it would take *p = r; and assume (it has no other info) that *p is a pointer to 8 bytes of memory. So the assignment merrily scribbles on 8 bytes, and the 4 bytes past the end of your char* pointer variable are gone.

The straight-forward assignment of malloc doesn't have this problem. The compiler sees 8 bytes returned in void* and sees a 4-byte char* and knows what to do to make it all work out.

Your compiler is correct, it is warning you about something which is dangerous.

While in general I agree (and why I spotted this problem before I even tried the code), there are plenty of functions in the standard library that take void * pointers and write or read other data types to them; such as fread, memcpy, bsearch etc and countless others. In fact, the C standard says that char pointers and void pointers will have the same storage and alignment requirements. Additionally, it says any void pointer may be converted to or from any incomplete or object type. (6.3.2.3). Addititonally, I don't care about anything other than systems with flat memory models (ie, POSIX)

What surprises me most is that the book is old (1993), and there are numerous reviews from respected authorities on C that never mention this issue. Also, the author claims to have tested all the code in the book on five different compilers from different vendors. Which is why I initially thought I was missing something.

5 compilers on the same architecture no doubt.

> Addititonally, I don't care about anything other than systems with flat memory models
The flatness (or otherwise) of the memory architecture is irrelevant to whether pointers have different representations. And I doubt POSIX cares about pointer representations either.


> there are plenty of functions in the standard library that take void * pointers
How many of them attempt to WRITE to a void** though, without knowing what the type the void* resolves to?

> In fact, the C standard says that char pointers and void pointers will have the same storage and alignment requirements
I'm sure that anyone who tries
double *blk;
if (alloc(&blk, 128) == true)
will feel comforted by knowing that.

Originally Posted by Salem

5 compilers on the same architecture no doubt.

The book covers issues from the points of view of DOS, Windows and Mac OS, and mentions UNIX, too. So I doubt.

> In fact, the C standard says that char pointers and void pointers will have the same storage and alignment requirements
I'm sure that anyone who tries
double *blk;
if (alloc(&blk, 128) == true)
will feel comforted by knowing that.

You're ignoring (or I have misunderstood) the bit of the spec I quoted that says that void pointers are specifically convertible regardless. (But diagnostic-generating, it would seem.)

My query is what am I missing from my understanding of what the author has written, given nobody else has ever raised the issue, in hundreds of discussions about the book online?

Originally Posted by rjek

You're ignoring (or I have misunderstood) the bit of the spec I quoted that says that void pointers are specifically convertible regardless. (But diagnostic-generating, it would seem.)

You've misunderstood. A double ** is not convertible to void **.

A pointer to void is special as it can be implicitly converted to any other pointer type (and vice versa).

However, a "pointer to pointer to void" cannot be implicitly converted to a "pointer to pointer to <another type>"

Question 4.9

He advocates this style because it does not combine two different types of result (in malloc's case, pointer and error condition) into one return value. I kinda buy that.

Arr, there're so many functions in standard library like malloc( fopen,...).... I don't see any advantage of this style.
Edit: It's quite funny to see that trying to make standard library 'safer'? is causing non-portable code.

Originally Posted by grumpy

However, a "pointer to pointer to void" cannot be implicitly converted to a "pointer to pointer to <another type>"

Yes. Which is why I said I spotted this before I even attempted to compile it, and asked what I was missing, given nobody else appears to have ever raised it.

malloc has a "binary" return status anyway; if it's not null, you win, if it is null, you lose.
Wrapping this up (however badly) into a function does no good.

Consider

Code:

char *p;
p = malloc( 10 );
strcpy( p, "hello" );

vs.

Code:

char *p;
alloc(&p, 10);
strcpy( p, "hello" );

The problem isn't the wrapper, the problem is the programmer NOT checking for errors when they should.

However, the former is at least predictable in that p will always be initialised regardless. So that if malloc does fail, at least you get a nice fat segfault for your trouble.

But case 2? Gee, that seems to leave the pointer UNINITIALISED!
I bet that's a barrel of fun trying to debug that fella.
Or is this an artefact of your "compact for brevity"?

This gets past the dodgy pointer assignment semantics, but still doesn't prevent sloppy coders from ignoring the return result. But at least they always get a valid pointer (or NULL).

Code:

void *alloc(bool *status, size_t z)
{
   void *r = malloc(z);
   *status = true;
   if (r == NULL) *status = false;
   return r;
}

The only 'valid' wrapper for malloc that I can think of is:

Code:

void *alloc(size_t size)
{
  void* mem;
  mem = malloc(size);
  if (!mem) {
    fprintf(stderr, "Out of memory!\n");
    abort();
  }
}

There are some good advices in "Writing Solid Code" unfortunately intermixed with some bad advice.