Refactor dbwrap_ctdb a bit

[Michael Adam]

On 2012-11-22 at 14:50 +0100, Volker Lendecke wrote:
> Hi!
> 
> Attached find a patch that refactors dbwrap_ctdb a bit. A
> while ago, the parse_record dbwrap backend operation was
> added. dbwrap_fetch() at the dbwrap toplevel was converted
> to use parse_record, so that the fetch backend operation
> became unnecessary. All but the dbwrap_ctdb backends were
> converted. dbwrap_ctdb however does a fake parse based
> internally on the fetch operation. This leads to allocations
> and memcpy operations where they are not strictly necessary.
> 
> The attached patchset converts dbwrap_ctdb to do a proper
> parse operation avoiding the memory allocations in the local
> cases.
> 
> Comments?

First of all: nice work! :-)

Some first comments on details:

- [PATCH 03/16] s3: Remove header==NULL code from db_ctdb_marshall_record

  I don't like that the fact that there is currently no caller
  that passes header == NULL into the function is used as a
  justificatino for removing all checks for header == NULL.
  We could at least add an early if (header == NULL) { return...  }
  guard.

  Currently, this is OK, but in the past, there were callers
  that handedin header == NULL.

- [PATCH 08/16] s3: Slightly simplify db_ctdb_marshall_loop_next

  Same comment as above. It makes it less obvious how to use the functions.
  This is not wrong currently, but gives me a bad feeling.

- [PATCH 05/16] s3: Add db_ctdb_ltdb_parse

  Wouldn't it be more natural to hand the db_ctdb_context into
  the function instead of a tdb_context?

  All callers hand in db->wtdb->tdb anyways.

Apart from these details, this I would of course push that set
after some tests.

Cheers - Michael

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 206 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20121123/9884754c/attachment.pgp>