I concur strongly with Derek's sentiments and would add some more comments:
I often use custom memory allocators to help debug wild pointer errors and
rather than zeroing out allocated memory, I will often write a repeating
multibyte sentinal value (such as 0xbaad) into the memory block so that
it's easy to tell from a memory dump that something accessed an
uninitialized memory block. When performance counts, this can be disabled
for release builds or replaced with memset(dest,0,count), but I rarely find
that initializing freshly allocated memory is a big performance bottleneck.
It can also be useful (at least in debug builds) to have xmalloc allocate
extra memory before the beginning and after the end of a requested block,
filling it with different sentinals and checking (in x_free) for pointers
writing off either end of the block. Finally, when I free a block, I
overwrite it with yet another sentinal, signifying freed memory, so that I
can easily identify wild-pointer bugs due to aliased pointers pointing to
freed memory (e.g., char *p, *q, c; p = malloc(10); q = p; strcpy(q,"foo");
free(p); c = *q;).
example (Caution: untested example code, not production quality):
static const char block_init[] = {'\xba', '\xad'};
static const size_t block_init_len = sizeof(block_init);
static const char block_header_padding[] = {'\x6b', '\x6b', '\x6b', '\x6b',
'\x6b', '\x6b', '\x6b', '\x6b',
'\x6b', '\x6b', '\x6b',
'\x6b', '\x6b', '\x6b', '\x6b', '\x6b' };
static const char block_end_padding[] = {'\xb6', '\xb6', '\xb6', '\xb6',
'\xb6', '\xb6', '\xb6', '\xb6',
'\xb6', '\xb6', '\xb6',
'\xb6', '\xb6', '\xb6', '\xb6', '\xb6' };
static const char block_padding_len = sizeof(block_header_padding);
static const char freed_block_stuffing[] = {'\xf0', '\x0f'};
static const size_t freed_block_stuffing_len = sizeof(freed_block_stuffing);
void *
xmalloc(size_t size)
{
size_t i = 0;
void * ptr = NULL;
char * dest = NULL;
#if defined(NO_ZERO_BYTE_ALLOCATIONS)
/* see my notes below on whether this should be treated as a bug */
assert(size > 0);
if (size == 0) {
char buf[80];
sprintf (buf, "internal domain error: attempt to allocate zero
bytes");
error (1, 0, buf);
}
#endif /* NO_ZERO_BYTE_ALLOCATIONS */
const size_t block_len = size + 2 * block_padding_len + sizeof(size_t);
ptr = malloc(block_len);
if (ptr == NULL) {
char buf[80];
sprintf (buf, "out of memory; can not allocate %lu bytes",
(unsigned long) block_len);
error (1, 0, buf);
}
/* initialize block with "uninitialized block" sentinal value */
for (i = 0, dest = (char *)ptr + sizeof(size_t) + block_padding_len;
i + block_init_len <= size; /* funky phrasing of comparison
* to avoid subtraction of unsigned
* values: avoid underflow errors
* if size < block_init_len!
*/
i+= block_init_len, dest += block_init_len) {
memcpy(dest, block_init, block_init_len);
}
/* stash nominal block length (without padding) for use in finding
tail padding in x_free */
*(size_t *)ptr = size;
/* initialize head and tail padding blocks to check for array index
overflow or underflow */
memcpy((char *)ptr + sizeof(size_t), block_header_padding,
block_padding_len);
memcpy((char *)ptr + sizeof(size_t) + block_padding_len + size,
block_end_padding, block_padding_len);
return (void *)((char *)ptr + sizeof(size_t) + block_padding_len);
}
void
x_free(void * ptr)
{
size_t i = 0, len = 0;
char * dest = NULL;
void * base = NULL;
assert(ptr != NULL);
if (ptr == NULL) {
char buf[80];
sprintf (buf, "attempt to free null pointer");
error (1, 0, buf);
}
base = (void *)( (const char *)ptr - sizeof(size_t) - block_padding_len)
len = *(const size_t *)base;
/* check for integrity of head and tail padding blocks */
if (memcmp( (const char *)ptr - block_padding_len,
block_header_padding, block_padding_len)) {
char buf[80];
sprintf (buf, "block header padding overwritten");
error (1, 0, buf);
}
if (memcmp( (const char *)ptr + len,
block_end_padding, block_padding_len)) {
char buf[80];
sprintf (buf, "block end padding overwritten");
error (1, 0, buf);
}
/* fill with freed-block sentinal... */
for (dest = (char *)base, i = 0;
i + freed_block_stuffing_len <= len; /* funky phrasing of
comparison
* to avoid subtraction
of unsigned
* values: avoid
underflow errors
* if len <
freed_block_stuffing_len!
*/
i+= freed_block_stuffing_len, dest += freed_block_stuffing_len) {
memcpy(dest, freed_block_stuffing, freed_block_stuffing_len);
}
free( (void *)((const char *)ptr - sizeof(size_t) - block_padding_len));
}
If code were written perfectly, we wouldn't have wild-pointer bugs and
these techniques would be unnecessary, but in the real world, we all see
wild-pointer errors and these ideas (adapted from Robert Ward's old (out of
print, alas!) book, "Debugging C") have served me quite well for many years.
As to non-portability, this code compiles without errors or warnings with
strict ANSI compiler switches.
As to the question of requests to allocate zero bytes, I would consider
those to be domain errors. A well-written program should not ask for
allocations of zero bytes. Following this line, I would write xmalloc to
consider such a request erroneous, and to generate a run-time error.
However, comments in the xmalloc code in CVS note that some places allocate
zero-byte blocks and free them. I grepped the code for "alloc *( *0" and
turned up nothing, so could someone clue me in on:
* Where zero-byte requests are made,
* Whether this represents good coding or sloppy coding,
* and Whether such requests should be considered bugs to be fixed?
Just My HO,
Jonathan
At 05:58 PM 5/2/00, Derek Scherger wrote:
> > > For safety, I propose that xmalloc zero out the memory it allocates. Any
> > > comments or rebuttals?
> > >
> > For safety, I would prefer it does not. I don't think we should
> > use non-portable solutions to cover up the faults of badly-written
> > software. I'd feel more comfortable using the software knowing that
> > some classes of errors would have been discovered.
>
>So you would rather have problems occur *randomly* depending on what the
>values *happen* to be at the moment? Personally, if something is not
>properly initialized I'd like a core dump every time so that it is found
>and fixed quickly.
===========================================================================
Jonathan M. Gilligan <[EMAIL PROTECTED]>
Research Assistant Professor of Physics (615) 343-6252
Dept. of Physics and Astronomy, Box 1807-B Fax: 343-7263
Stevenson Center 6823
Vanderbilt University, Nashville, TN 37235 Dep't Office: 322-2828
