Re: trying to make my "fillvalues" function work...

Martin Jørgensen schrieb:
Michael Mair wrote:
[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);
}
}
]

As pointed out elsethread:
- This does not take care of the already allocated memory,
open files and other resources.

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.

- Generating a helpful, ideally unique location string to
facilitate debugging may be complex so that the generation
of the location string should ideally take place within
the if (pointer == NULL) case, i.e. outside of
checkPtr_error
- You do not gain anything substantial by using this function
instead of some kind of dedicated debug/error output function.

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 <ln1wxn14x4.fsf@xxxxxxxxxxxxxxx> 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;
}

--
E-Mail: Mine is an /at/ gmx /dot/ de address.
.

Relevant Pages

  • Re: sizeof(ptr) = ?
    ... The value returned by malloc() is of type 'void*', ... The memory is typeless until an object has been written ... Since 'void' is defined to be an incomplete ... an lvalue of a complete type, there must be a pointer conversion ...
    (comp.lang.c)
  • Re: Policy on rebooting?
    ... >> appropriate sizes of the pools statically was impossible. ... The problem is not the use of dynamic memory, ... so that the allocation can be ... If you can guarantee that only one pointer to a particular block is ...
    (comp.arch.embedded)
  • Re: various objects in my VB6 project - Calling IUnknown
    ... memory allocation and deallocation is half ... Doing something like this with a pointer would be a permanent leak in C++. ... Visual C++ has heap checking built ...
    (microsoft.public.vb.general.discussion)
  • Re: Linked List & Dynamic Memory Allocation
    ... you have a memory leak. ... that storage will not be available any longer. ... You must call freefor every pointer you got via malloc. ... There are three serious errors you can make using dynamic allocation: ...
    (microsoft.public.vc.mfc)
  • Re: allocating mem in a function and assigning a ptr to the first byte of that mem array...
    ... Anway the idea is that there's a func Test which takes a pointer to ... void argument, allocate some memory, set this memory with some cotent, ... int main(int argc, char **argv) ...
    (comp.lang.c)