Hi Emeric, > Aren't you afraid by potential side effects using this approach?
On systems with overcommit (like Linux by default) malloc() will most likely return a non-NULL pointer even if there is currently no physical memory available. Processes will just get killed by the OOM killer if that's the case. But that may be different on other platforms or with changed kernel configs, so this might have gotten slightly more problematic since the charon daemon was originally started. > You may get a NULL pointer and perform some operations with it, like pointer > arithmetic, without crashing. Sure but I don't see our code doing stuff like that. In most cases there will be an immediate segmentation fault (e.g. via INIT macro where the pointer is dereferenced to initialize the struct or if the allocated memory is used in memset/memcpy/snprintf). > Furthermore you could possibly have security issues before eventually crash. Maybe but as mentioned malloc() returning NULL will almost certainly result in a segmentation fault in our codebase. > The question is: since you have done the job to get proper malloc hooks with > the leak detective, why not just abort on failure? I hope you are not enabling leak detective on your production systems. It's quite a hack and has a huge performance impact due to the single lock. Most allocations are wrapped in calls to the INIT* macros (~1250). These could theoretically be modified to handle situations where NULL is returned. However, some of these calls are in library code where it might be more appropriate to just return NULL too or OUT_OF_RES instead of aborting the process, thus, forcing the caller to handle this, which would require a lot more code (one of the reasons this hasn't been done so far). There are also about 190 direct calls to malloc(), about 40 to realloc() and about 20 to calloc() in our code. These would all have to be handled individually (maybe replaced by a macro). But as said, a segmentation fault will be caused there if NULL is returned anyway, so I'm not sure if it's worth changing any of this. Regards, Tobias _______________________________________________ Dev mailing list [email protected] https://lists.strongswan.org/mailman/listinfo/dev
