should pysrp check malloc returns?

[radii]

The C implementation in _srp.c calls malloc and assumes that allocations succeed, which is not guaranteed by the C standard.

Now, there are a couple of possible responses to this observation:

• perhaps the community standard for Python extensions says that malloc never fails -- I don't know, I have never written a Python module in C.
• perhaps having the app immediately SEGV is the proper response to malloc failing. Certainly I've never seen a Python application that could deal with allocations failing. I haven't done an exhaustive survey but everywhere I checked, malloc returning NULL results in an immediate NULL pointer dereference, and I think Linux these days is protected against allocating things at address 0.
• we could switch to a malloc wrapper which explicitly guarantees that failed allocations crash the app with a reasonable error message rather than a potentially obsure SEGV.
• or, perhaps we do want to handle failed malloc and should modify the ~17 callsites in _srp.c to test for NULL and return errors to their callers.

Thoughts?

[cocagne]

Thanks for giving me the benefit of the doubt on this one radii, but the failure to check malloc() returns is a definite bug in the code. According to Python's C-API documentation, the appropriate response to memory allocation failures is to clean up and return from the extension function with: "return PyErr_NoMemory();" In fact, it might be better to switch entirely over to the PyMem_Malloc() and PyMem_Free(). I'd initially planned on keeping csrp and pysrp close to identical to ease maintenance burdens but they've already diverged somewhat as it is. The memory allocation checks, for example, are already in csrp and, for whatever reason, I never thought to put equivalent ones into pysrp.

Thanks for catching this.

Tom

On Sun, Sep 28, 2014 at 1:28 AM, radii notifications@github.com wrote:

The C implementation in _srp.c calls malloc and assumes that allocations succeed, which is not guaranteed by the C standard.

Now, there are a couple of possible responses to this observation:

• perhaps the community standard for Python extensions says that malloc never fails -- I don't know, I have never written a Python module in C.
• perhaps having the app immediately SEGV is the proper response to malloc failing. Certainly I've never seen a Python application that could deal with allocations failing. I haven't done an exhaustive survey but everywhere I checked, malloc returning NULL results in an immediate NULL pointer dereference, and I think Linux these days is protected against allocating things at address 0.
• we could switch to a malloc wrapper which explicitly guarantees that failed allocations crash the app with a reasonable error message rather than a potentially obsure SEGV.
• or, perhaps we do want to handle failed malloc and should modify the ~17 callsites in _srp.c to test for NULL and return errors to their callers.

Thoughts?

— Reply to this email directly or view it on GitHub https://github.com/cocagne/pysrp/issues/10.