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
#10.