PATCHES: Password sync as active directory domain controller

Alexander Bokovoy ab at samba.org
Fri Jul 22 13:03:34 UTC 2016


On Fri, 22 Jul 2016, Stefan Metzmacher wrote:
> Hi Andrew,
> 
> > We can't have a grep of files in /proc in Samba.  Sorry.
> > 
> > If you want mutual exclusion between the scripts, can you use a
> > transaction lock over the ldb, or fcntl locks on another file?
> 
> I'm using fcntl locks now.
> 
> > Otherwise, the main thing I see is that autobuild doesn't run with --
> > with-gpgme.  
> 
> It does it's autodetected if libgpgme11-dev is installed.
> 
> > Finally, after the https://evil32.com/ episode, we should not accept 8-
> > char key ids.  Please update the docs to only suggest 16-char IDs, and
> > make the code refuse to accept < 16-char IDs.
> 
> Fixed.
> 
> > Otherwise, I'm finally OK with all this.  There is still a lot of code
> > here, and some of it is pretty dense, but these are the only objections
> > I have so far.  I really appreciate the efforts you have gone to to
> > address my concerns.
> 
> https://git.samba.org/?p=metze/samba/wip.git;a=shortlog;h=refs/heads/master4-gpgme
> just needs review markers.
You can add my Reviewed-by to:
fff8e36c397bd3ba8a1d3d74da79ad6982746d7f
4530de4d0358f3fa1cd1384dde520e4949333846
6edad4f74304f8b77346fbae0516c3960f0966c1
451c8010ec5ca4bc3c70fe2f8e9e903dc5fcb116
9874cf63f67ccc65f3078d68a8f63c80b586a2b7
161a130f4d1b6f24f60c76ad7054228cc0d0c6d3
fb00f17ec6036c463c9a191742841b19f4078128
1755e4f1fdd359695299d454a79fb42bfbc496a5
3071b3ce4f1840c9b6b1b264e491c0b56c86cd93
a30928f5ede5591a366dcddf5fe32406984f24c3
e3804875ecad901af462fa95c3f007c27a7f9d7d
464fe0f424f6885fbfd55929142b43d02699c75f
c886a1a98dd4deaf750cef0be1b84f17f17fe571
bfb62545ea3607631c4ce41974f8e69231a6918a
c979e58b125d58d71fba65108712c14d46b609cb
3cfdeb6f770b11a51146cbcf7b458d57867f609c

In 67de4260be3a5be0528cc355c267113f2529d43c 'pass' is not needed in the
import gpgme exception handling. This is minor but you have 'pass' in
all exceptions. 'pass' is only needed if you don't have any other
statement there. Please fix it and then you can add my Reviewed-By here.

Same in 8bfc13f2f3f2c6c8adb867acea68ca2c29d9bd05, in
check_current_pid_conflict() there is no need for 'pass':
+            try:
+                fcntl.lockf(self.lockfd, fcntl.LOCK_EX | fcntl.LOCK_NB)
+                got_exclusive = True
+            except IOError as (err, msg):
+                if err != errno.EACCES and err != errno.EAGAIN:
+                    log_msg("check_current_pid_conflict: failed to get exclusive lock[%s] - %s (%d)" %
+                            (self.lockfile, msg, err))
+                    raise
+                pass

However, next exception needs 'pass' because it is the only statement
under 'except':
+                try:
+                    self.current_pid = int(buf)
+                except ValueError as e:
+                    pass

Same in update_pid().

Finally, in the run() itself:

+                try:
+                    self.samdb = self.connect_system_samdb(url=self.samdb_url)
+                except Exception as msg:
+                    self.samdb = None
+                    log_msg("Connect to samdb Exception => (%s)\n" % msg)
+                    if wait is not True:
+                        raise
+                    pass
+
+            try:
+                sync_loop(wait)
+            except ldb.LdbError as (enum, estr):
+                self.samdb = None
+                log_msg("ldb.LdbError(%d) => (%s)\n" % (enum, estr))
+                pass

these two 'pass' are not neccessary.


-- 
/ Alexander Bokovoy



More information about the samba-technical mailing list