[SCM] Samba Shared Repository - branch master updated

[Jeremy Allison]

On Tue, Mar 06, 2018 at 01:40:02AM +0100, Andrew Bartlett wrote:
> The branch, master has been updated
>        via  4014499 ldb_tdb: Build a key value operation library
>        via  e3d364a partition: Allow a different backend store from @PARTITION
>        via  4d5180e ldb_tdb: Implement a traversal function in key value ops
>        via  c9f2ff2 ldb_tdb: Use key value ops for fetch command
>        via  68423e9 ldb_tdb: factor out the (to be) common init code
>        via  141148e ldb_tdb: Add errorstr to the key value ops
>        via  c66a005 ldb_tdb: Remove tdb_get_seqnum and use a generic 'has_changed'
>        via  e21a476 ldb_tdb: Add lock_read and unlock_read to key value ops
>        via  448101a ldb_tdb: Replace tdb transaction code with generic key value ones
>        via  e33fb2c ldb_tdb: Replace exists, name and error_map with key value ops
>        via  9c8f00a ldb_tdb: Begin abstracting out the base key value operations
>        via  2a0e022 dsdb: The schema should be reloaded during the transaction
>        via  8ac1646 samdb/schema_load: do schema loading with one search
>        via  313b0c6 schema_set: Add a missing newline between functions
>        via  bca8ac0 remove_dc.py: Abort transaction before throwing an exception
>        via  c4c64ff ldb_mod_op_test: Fix core dump on ldb_case_attrs_index_test_teardown
>        via  242cf33 partition: Leave metadata.tdb unlocking until last
>        via  8caf84a schema: Do not read different schema sequence values during a read transaction
>        via  1265346 partition: Use a transaction to write and a read lock to read the LDB_METADATA_SEQ_NUM
>       from  c59d5e1 build: fix standalone ctdb build --with-systemd
> 
> https://git.samba.org/?p=samba.git;a=shortlog;h=master

One quick (random) comment on this patchstream, which
I didn't get to in time:

> commit c66a0054fb99b6519344c11954417d6615e1310b
> Author: Garming Sam <garming at catalyst.net.nz>
> Date:   Tue Jan 10 23:23:22 2017 +1300
> 
>     ldb_tdb: Remove tdb_get_seqnum and use a generic 'has_changed'
>     
>     Signed-off-by: Garming Sam <garming at catalyst.net.nz>
>     Reviewed-by: Andrew Bartlett <abartlet at samba.org>

This one adds the function:

+static bool ltdb_tdb_changed(struct ltdb_private *ltdb)
+{
+       bool ret = (tdb_get_seqnum(ltdb->tdb) != ltdb->tdb_seqnum);
+
+       ltdb->tdb_seqnum = tdb_get_seqnum(ltdb->tdb);
+
+       return ret;
+}

which calls tdb_get_seqnum() *twice*. Now it (probably)
doesn't matter, but *theoretically* you could have
the value of tdb_get_seqnum() change between the two
calls. You're only storing the latest value anyway,
so arguably you want the latest version stored here,
but it just seems sloppy to do the two calls here.

Shouldn't this be:

uint32_t seq = tdb_get_seqnum()

and then use 'seq' in both places you're currently
calling tdb_get_seqnum() ?

Jeremy.