[ditrack commit] r1617 - src/trunk/DITrack/DB

[Ivan Glushkov]

Hi.
I haven't understood you.
> What is the possible scenario that would render "lockfd" unbound?
Does it mean "when we have uninitialized self.lockfd"?
Now it may become uninitilized if Database.__init__() failes
(reasons:
             raise DITrack.DB.Exceptions.NotDirectoryError
             raise DITrack.DB.Exceptions.NotDatabaseError
             raise DITrack.DB.Exceptions.InvalidVersionError
)

To tell the truth, the purpose of this commit was to enable moving  
lock/unlock functions to separate module.
It was impossible without slight modification of the current functionality.
It will be entirely rewritten in the nearest future. ;)


On Mon, 21 May 2007 00:00:33 +0400, Vlad Skvortsov <vss at 73rus.com> wrote:

> Hmm, can you give some explanations regarding this commit? What is the
> possible scenario that would render "lockfd" unbound?
>
> In general, my own preference is to have as few conditionals as possible
> (to test as few executions paths as possible), so if there is no
> compelling reason to have this functionality, I'd rather want to remove
> these.
>
> gli at ditrack.org wrote:
>> Author: gli
>> Date: 2007-05-20 03:49:11 -0700 (Sun, 20 May 2007)
>> New Revision: 1617
>>
>> Modified:
>>    src/trunk/DITrack/DB/Common.py
>> Log:
>> function unlock() now work with the file descriptor through arguments
>>
>> Modified: src/trunk/DITrack/DB/Common.py
>> ===================================================================
>> --- src/trunk/DITrack/DB/Common.py	2007-05-18 20:41:35 UTC (rev 1616)
>> +++ src/trunk/DITrack/DB/Common.py	2007-05-20 10:49:11 UTC (rev 1617)
>> @@ -167,7 +167,8 @@
>>
>>      def __del__(self):
>>          # Unlock database
>> -        self.unlock()
>> +        if hasattr(self, "lockfd"):
>> +            self.unlock(self.lockfd)
>>
>>      def __getitem__(self, key):
>>          """
>> @@ -382,7 +383,6 @@
>>              os.mkdir(datadir, 0755)
>>
>>          lock_fname = os.path.join(datadir, LOCK_FILE)
>> -        self.lockfd = None
>>
>>          if not os.path.exists(lock_fname):
>>              # if there's no lock file - create it and lock it
>> @@ -518,11 +518,11 @@
>>          # contained in the LMA issue
>>          self.lma.remove_comment(issue_id, comment_name)
>>
>> -    def unlock(self):
>> +    def unlock(self, lockfd):
>>          # if database path is wrong, object doesn't have lockfd  
>> attribute
>> -        if hasattr(self, "lockfd") and self.lockfd:
>> -            fcntl.flock(self.lockfd.fileno(), fcntl.LOCK_UN)
>> -            os.unlink(self.lockfd.name)
>> +        if lockfd:
>> +            fcntl.flock(lockfd.fileno(), fcntl.LOCK_UN)
>> +            os.unlink(lockfd.name)
>>
>>
>>  class Filter:
>>
>> _______________________________________________
>> Commit mailing list
>> Commit at lists.ditrack.org
>> http://lists.ditrack.org/mailman/listinfo/commit
>>
>
>