To: "Stefan (metze) Metzmacher" <metze@samba.org>
Cc: Rusty Russell <rusty@rustcorp.com.au>,
    Volker Lendecke <Volker.Lendecke@SerNet.DE>,
    Jeremy Allison <jra@samba.org>,
    Samba Technical <samba-technical@lists.samba.org>
Subject: Re: Talloc: pool optimizations
In-Reply-To: <4D99E18D.7080708@samba.org>
References: <4D94C542.5050503@samba.org>
	<87fwpybkjk.fsf@rustcorp.com.au>
	<4D99E18D.7080708@samba.org>
X-Mailer: VM 8.1.0 under 23.1.1 (x86_64-pc-linux-gnu)
Reply-To: tridge@samba.org
FCC: ~/Mail/SMAIL.in
--text follows this line--
Hi Metze,

 > Is it ok to push everything without TODO in the commit message?

yes, although you may wish to incorporate a few minor changes from the
comments below:

- in the TC_POOL_SPACE_LEFT() macro, I think it would be clearer to
  use the PTR_DIFF() macro. That uses ptrdiff_t which I think is the
  preferred way to do pointer differencing in C

- in macros, it is generally a good idea to brace the arguments, for
  example in:

    #define TC_POOL_FIRST_CHUNK(pool_tc) \
    	((void *)(TC_HDR_SIZE + TALLOC_POOL_HDR_SIZE + (char *)pool_tc))

  I think it would be better to use '(pool_tc)' on the right hand side,
  instead of 'pool_tc'.  This avoids tricky bugs later when someone else uses
  the macro with an expression as an argument. (there are quite a few
  macros in the patches that could be improved in this way)

- the patches use void* a bit more than I would like. For example, it
  may be better for TC_POOLMEM_NEXT_CHUNK() to return a
  'struct talloc_chunk *' rather than a void*. That would give the
  code more type safety, and perhaps clarity. It may make things
  easier to make some of the macros be static functions instead, which
  the compiler should inline.

- UNFINISHED COMMENTS