Martin said:
[context: Using
void checkPtr_error(const char *location, const void *pointer)
{
if (pointer == NULL) {
fprintf(stderr, "Memory allocation failure (%s). Bye\n",
location ? location : "Unknown location");
exit(EXIT_FAILURE);
}
}
]
My mallocs happens in the beginning of the program so no files or other
resources are opened at this point. The already allocated memory is
free'd when the program closes and if it can't get the memory it needs,
then it can't do anything about that at all and therefore I don't see
any problem with this method.
Do you handle the "already allocated memory" with atexit() to
be on the safe side? If you do: Yes, that takes care of it.
Otherwise, you are only _hoping_ that it is taken care of.
In your scenario (malloc() first) this may suffice.
However, you may find yourself calling the respective function
recursively or whatever -- if you then change your error
handling to returning an error in a less disruptive manner,
then you may still bind memory other processes might have
used. Or your programme becomes one or several library
functions and you can no longer die "just like that", then
returning an error but forgetting to free the memory may eat
up the storage slowly.
Apart from that: If you leave out calls to free() in the above
scenario, you may be tempted to leave them out in principle.
However, the programme-becomes-library thing applies much more
in this case; in addition, in my experience free()ing helps
find more errors than checking whether malloc() returned a
null pointer -- often writing over the boundaries of the
allocated storage makes itself felt when calling free().
As long as your operating system or implementation does not
_guarantee_ you that allocated storage is taken care of as if
you had free()d it explicitly, I'd rather explicitly free()
the storage.
I don't understand that viewpoint. I gain that substantial effect, that
I don't overwrite anything in memory because the program shuts itself
down before it tries to do anything stupid. Moreover, I get to know
exactly which malloc caused the error so I can see if I made any errors
at that point.
If you malloc() all the memory essentially at once, the information
is not as valuable as you may think.
In this case you can as well go for
a = malloc(asize * sizeof *a);
...
foo = malloc(foosize * sizeof *foo);
if (!(a && ... && foo)) {
/* you can still find out the pointer where it first went wrong */
/* handle error */
}
without loosing anything. In fact, calling your checkPtr_error()
inside the fail branch for all returned pointers is perfectly
acceptable in my book as there is only a slight waste of time
in case of failure.
Keep the null pointer checks together with the malloc()s; see
Keith's CHECKED_MALLOC from <
[email protected]> which
does the same job as your suggestion with only one function/macro
call and keeps the checks together.
Cheers
Michael
PS: As a macro is used anyway, I'd consider going all the way:
#include <stdlib.h>
#include <stdio.h>
/* belongs to some kind of "debug.c" */
#include <stdarg.h>
int DebugErrPrintf(const char *format, ...)
{
int ret;
va_list arglist;
va_start(arglist, format);
ret = vfprintf(stderr, format, arglist);
va_end(arglist);
return ret;
}
int DebugTracePrintf(const char *format, ...)
{
return 0;
}
/* end "debug.c" */
void *checked_malloc(size_t size,
const char *optInfo)
{
void *result = malloc(size);
if (result == NULL) {
DebugErrPrintf("%s malloc(%lu) failed\n",
optInfo ? optInfo : "", (unsigned long)size);
exit(EXIT_FAILURE);
}
DebugTracePrintf("%s malloc(%lu) ok\n",
optInfo ? optInfo : "", (unsigned long)size);
return result;
}
#define STRINGIZE(s) #s
#define XSTR(s) STRINGIZE(s)
#define CHECKED_BYTENO_MALLOC(dest, size) \
((dest) = checked_malloc((size), \
#dest ":" \
__FILE__ ":" \
XSTR(__LINE__) \
))
#define CHECKED_ARRAY_MALLOC(dest, size) \
CHECKED_BYTENO_MALLOC(dest, (size) * sizeof *(dest))
int main(void)
{
double *dptr;
void *vptr;
CHECKED_ARRAY_MALLOC(dptr, 42);
printf("dptr = %p\n", (void *)dptr);
free(dptr);
CHECKED_BYTENO_MALLOC(vptr, (size_t)-2);
printf("vptr = %p\n", vptr);
free(vptr);
return 0;
}
