|
From: | grischka |
Subject: | Re: [Tinycc-devel] tcc_relocate() and tcc_relocate_ex() |
Date: | Tue, 14 Aug 2012 14:02:51 +0200 |
User-agent: | Thunderbird 2.0.0.24 (Windows/20100228) |
Sean Conner wrote:
It was thus said that the Great grischka once stated:Sean Conner wrote:As to the patch I think that + s1->runtime_mem = NULL; would break tcc_relocate and I don't see why you need it.I did that so that the call to tcc_delete() wouldn't crash when it tried to free s->runtime_mem that might contain a garbage value (since tcc_relocate_ex() would never set that field). Good thing too, because tcc_new() *might not* be initializing NULL pointers correctly [1].Well, from my point of view that line would not fix tcc_new but cause a memory leak for tcc_relocate.I did a bit more work. The assignment of s1->runtime_mem to NULL does not lead to any memory leaks. On systems where the NULL pointer *is* all zeros (most, if not all, modern systems I am aware of) removing the assignment does not cause a crash (since the memory used to store the state is zeroed out). On systems where the NULL pointer *IS NOT* all zeros [1], removing the assignment *WILL CRASH*. Not "might crash", *WILL CRASH*. [2]
Well, behavior of free for invalid values is undefined. It might crash but might as well restart the computer or do nothing or ... Anyway, features and bugs are not different in that both are generated as side-effects from the written lines of code. I can't help and sorry and all but the feature that you wrote here is ... a 100% state of the art memory leak. However the most grave argument against this line is that it breaks assumptions, here the assumption that tcc_new *DOES* the right thing with initialization. You never ever put code to implement safety precaution against the failure of assumptions made elsewhere in the same program. See, on a system where memset doesn't clear pointers tcc or libtcc would fail already in tcc_new. It would never reach your "safety" line. --- grischka
[Prev in Thread] | Current Thread | [Next in Thread] |