[SCM] Samba Shared Repository - branch master updated

Garming Sam garming at catalyst.net.nz
Wed Mar 7 00:35:48 UTC 2018


I noticed this afterwards, and in theory it shouldn't be able to change,
but it's better to just do it once. Patch attached.

tdb_get_seqnum also seems to return an int.

Cheers,

Garming

On 07/03/18 11:38, Jeremy Allison wrote:
> 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.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-ldb_tdb-Remove-unnecessary-call-to-tdb_get_seqnum.patch
Type: text/x-patch
Size: 920 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20180307/b4edeb64/0001-ldb_tdb-Remove-unnecessary-call-to-tdb_get_seqnum.bin>


More information about the samba-technical mailing list