[SCM] Samba Shared Repository - branch master updated

[Andreas Schneider]

On Wednesday, 7 March 2018 01:35:48 CET Garming Sam via samba-technical wrote:
> 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.

Could you also please use 'bool has_changed' instead of 'bool ret'?

> 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.


-- 
Andreas Schneider                   GPG-ID: CC014E3D
Samba Team                             asn at samba.org
www.samba.org