Mailing List archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[vdr] Re: vdr 1.3.20: using of free'd memory



syphir@syphir.sytes.net(C.Y.M)  10.02.05 23:05

Once upon a time "C.Y.M " shaped the electrons to say...

>Stefan Huelswitt wrote:
>> Hi,
>> in recording.c cRecording::ScanVideoDir() is a place where
>> already free'd memory is used:
>>
>> free(buffer);
>> buffer = ReadLink(buffer);

That's the reason why i vote for:
After every "free" or "delete" or simpliar "invalations" the pointer 
should be set to NULL.
Without ANY discussion/thoughts if it makes sense in this particular
case. It's not worth to try hand optimize by doing -usually undocumented-
assuption. But the chance to use an invalid pointer or leak
memory is signifanctly reduced because it crashes very early.
One can use his brain for more complex thoughts than wasting it for
"hand optimizing" code ;-)

In this case it would have looked like:

free(buffer);
buffer=INVALID;
buffer = ReadLink(buffer);

If not the programmer himself is stumpling other that "funny"
looking code, "ReadLink" would
complain about, and he hopefully would not just remove the
"superflous" buffer=INVALID; line...

Too tools like "lint" or "boundschecker" would detect such
places in seconds.


>> IMO this should be like:
>>
>> char *old=buffer;
>> buffer = ReadLink(old);
>> free(old);
   old=INVALID; // just to get used to.
>>


Maybe the definition of "INVALID" might be worth a discussion...
some prefer 
#define INVALID NULL
what's generally faster.
Others setting them to the programmers initials, maybe
#define INVALID (void *)0xC0DE727A
to make it easier to trace back who invalidated the pointer,
while other programmers are still using it...

or 
#define INVALID (void *)0x0badC0DE



Rainer






Home | Main Index | Thread Index