[Discuss] Casting the return value of malloc bad?

[David Bronaugh]

pw wrote:
> Noel Burton-Krahn wrote:
>> Without pomposity, here's what works for me:
>>
>> int err = -1;
>> MyType *p = 0;
>>
>> do {
>>    /* allocate memory and use a debug library to detect leaks */
>>    p = malloc(sizeof(*p));
>>    debug_leak_create_assertb(p);
>>
>>    /* do some more actions with error checking */
>>
>>    err = 0;
>> } while(0);
>>
>> /* clean up on error */
>> if( err ) {
>>
>>    debug_leak_free(p);
>>    free(p);
>> }
>>
>> (1) I like the "do { /* lots of error checking */ err=0; } while(0);" 
>> construct because it avoids a lot of nested if's like "if(malloc) { 
>> if(open) {} if(read) { } else {} else {} else {}, etc."  I find 
>> nested if/else blocks really hard to read, and the cleanup-on-error 
>> code gets really scattered.
>>
>> (2) I have a debug library that defines macros to track allocated 
>> resource by the source file and line number they were allocated at.
>>
>> #define debug_leak_create_assertb(x) \
>>    if( !(x) ) { /* error message about x */; break; }
>>    debug_leak_record((x), __FILE__, __LINE__)
>>
>> #define debug_leak_free(x) \
>>    if( (x) )  { debug_leak_forget((x)); (x) = 0; }
>>
>> During debugging, I periodically log the total number of allocations 
>> by line number.  If there's a leak, the allocations at one particular 
>> line will constantly increase, so it's easy to find out what's 
>> leaking and where it starts.  My library also complains when 
>> something is freed twice.
>>
>>
>> --Noel
>
> I also like code that defines variables beyond the
> basic memory allocation. This allows applications
> to perform tests on allocations to perform operations
> based on variable type as well as other variable attributes
> that are not provided by simply malloc'ing storage.
>
> ie:
>
>
> #define STORAGE_TYPE_NOT_ALLOCATED 0x00000000
> #define STORAGE_TYPE_UCHAR 0x00000001
> #define STORAGE_TYPE_USHORT 0x00000002
> etc...
> .
> .
> .
> #define STORAGE_TYPE_EXTREMELY_COMPLEX 0x00000010
>
> #define MAX_NAME 1024;
>
> typedef union
> {
>     void *pv;
>     unsigned char *puc;
>     unsigned short *pus;
>     float *pflt;
>     double *pdbl;
>     .
>     .
>     .
> }STORAGE;
>
> typedef struct __VARIABLE
> {
>     uint64 variable_id;
>     uint64 variable_parent;
>     char variable_name[MAX_NAME];
>     long variable_type;
>     STORAGE storage;
>     VARIABLE *sub_variable;
>     .
>     .
>     .
>     
> }VARIABLE;
>
>
> VARIABLE_LIST *GetVariableByStorageType( VARIABLE_LIST *variable_list)
> {
> ........
> }
>
> VARIABLE *GetVariableByID(VARIABLE_LIST *variable_list)
> {
> ....
> }
>
> VARIABLE *GetVariableByName(char *name)
> {
> ......
> }
>
> VARIABLE *MakeStorage(long amount, long storage_type)
> {
> /*set up VAIABLE STORAGE */
> ....
> }
>
> VARIABLE *AddSubstorage(VARIABLE *variable, long sub_storage_type)
> {
> ......
> }
>
> int DiscardStorage(VARIABLE *variable)
> {
> /*get rid of storage and substorage*/
> ................
> }
>
>
> int CheckForStorageOverflow(...)
> {
> ........
> }
>
> long WhatTypeOfStorageIsThisVariableSupposedToBe(VARIABLE *variable)
> {
> ......
> }
>
> int DoSomethingBasedOnType(VARIABLE *variable)
> {
>     int ret_val=0;
>
>     switch(variable->variable_type)
>     {
>         case STORAGE_TYPE_NOT_ALLOCATED:
>         ...
>         case STORAGE_TYPE_UCHAR:
>         ...
>         case ...
>         default:
>         ....
>     }
>
>     return ret_val;
> }
>
>
> Peter
The point of this code escapes me entirely.

David