[openssl-users] Cleanup and changing the malloc routines

[Salz, Rich]

1. I trust that functions to allocate specific data 
  types (such as bignums) are not changed by this.
	They are not.

2. Macros that take zero arguments (such as 
  "OPENSSL_malloc_init()") are not accepted by all 
  compilers.  In my own API designs this has forced 
  me to give such macros a dummy argument (which is 
  never used, it just pleases the preprocessor).
	Luckily, we are limiting to c89, so this isn't an issue.

3. When compiling without compile-time debug flags, 
  the various debug arguments should not be passed 
  (or otherwise referenced) by the macros, avoiding 
  a bunch of "foo.c" and line number values from 
  filling up the compiled binary.  Something like
	This is a good idea.  In fact, I'll make all the memory-debug stuff disabled via the #ifdef.

4. For the basic functions (not the callbacks), there 
  seems to be no reason not to change to size_t now.
	Agreed.  It will happen in 1.1, just not the planned commit.

5. Given the huge number of backward incompatibilities 
  in OpenSSL 1.1.x, this seems a good time to change to 
  size_t in the callbacks too.
	Yup, same reason.

6. Maybe there should be an alternate get/set_mem_functions 
  with non-linenumber callbacks (behind the scenes, either 
  would set the the other callbacks to stubs that add/remove 
  the extra args, the initial defaults would be direct for 
  speed and to support the "defer to default implementation" 
  scenarios).
	If you look, that's what the current (undocumented) code does.  It was really hard to understand.  I think being able to replace malloc/realloc/free at runtime, as opposed to editing the source and building your own version, is the wrong trade-off to make.  But that's the goal of this thread:  to find out if anyone is actually doing that.

7. Maybe the get/set_mem_functions should (in their debug 
  variant) should also be able to change the backend for 
  debug_push/_pop.
	Same as #6.

8. For readability, defines with omitted (opaque) 
  definitions should be documented with elided args such 
  as #define OPENSSL_malloc(num) ... and 
  #define CRYPTO_MEM_CHECK_ON 0x... to indicate that they 
  do not (typically) expand to nothing/typically do expand 
  to a numeric constant
	That seems reasonable; anyone else agree?

9. Typo: "pointers for with the current functions"
	Thanks!

10. It would be helpful to clarify which older functions are 
  getting removed.  It's a bit like reading a patch without 
  the removed lines.  Kind of impossible to tell what you 
  propose to remove.
	Here's the internal git commit message.  Do you need/want more detail?
    Make the "change wrapper functions" be the only paradigm.
    Wrote documentation!
    Format the 'set func' functions so their paramlists are readible
    Format some multi-line comments.
    Remove LEVITTE_DEBUG_MEM
    Remove ability to get/set the "set options" functions at runtime.
    Remove ability to get/set the "memory debug" functions at runtme.
    Remove CRYPTO_pop_all
    Remove MemCheck_* and CRYPTO_malloc_debug_init macros
    Remove memleak_callback stuff.
    Remove {OPENSSL,CRYPTO}_remalloc.
    Remove {OPENSSL,CRYPTO}_strdup; use BUF_strdup.
    Add CRYPTO_mem_debug(int flag) function.
    Add test/memleaktest.
    Rename CRYPTO_dbg_xxx to CRYPTO_mem_debug_xxx
    Rename CRYPTO_{push,pop}_info to CRYPTO_mem_debug_{push,pop}
    Rename CRYPTO_malloc_init to OPENSSL_malloc_init; remove needless calls.
    Rename *_realloc_clear to *_clear_realloc, like *_clear_free
    Only two macros CRYPTO_MDEBUG,CRYPTO_MDEBUG_ABORT to control this.




Enjoy (NOT A CONTRIBUTION)

Jakob
-- 
Jakob Bohm, CIO, Partner, WiseMo A/S.  http://www.wisemo.com
Transformervej 29, 2860 Søborg, Denmark.  Direct +45 31 13 16 10
This public discussion message is non-binding and may contain errors.
WiseMo - Remote Service Management for PCs, Phones and Embedded