[SCM] Samba Shared Repository - branch master updated
Andreas Schneider
asn at samba.org
Wed Mar 7 09:36:24 UTC 2018
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
More information about the samba-technical
mailing list