PATCHES: Password sync as active directory domain controller
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.
> > 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.
> just needs review markers.
You can add my Reviewed-by to:
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':
+ 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))
However, next exception needs 'pass' because it is the only statement
+ self.current_pid = int(buf)
+ except ValueError as e:
Same in update_pid().
Finally, in the run() itself:
+ 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:
+ except ldb.LdbError as (enum, estr):
+ self.samdb = None
+ log_msg("ldb.LdbError(%d) => (%s)\n" % (enum, estr))
these two 'pass' are not neccessary.
/ Alexander Bokovoy
More information about the samba-technical