David Douthitt writes: > | >>> Clemens Hintze <c.hintze / gmx.net> 03/20/00 11:08PM >>> ... > | Perhaps it is what you have searched? > | > | But then, nobody has told me if you like to re-invent the wheel again > | ;-) > > Only sometimes :-) And here you have found the *great* danger of Ruby... because programming in Ruby is so fun, and beause Ruby makes it really easy to re-invent the wheel again and again, I often do it! ;-) Instead of using smart code snippets from other, I really like to write them myself. Not every time, but if I am bored, for example. I never had that feeling with other languages... :-) > Yet, I don't think your script is what I had in mind. I'm not looking > to "lock a file" - but to create a UNIX-style lockfile in > (typical example) /var/spool/locks/ - as it is under HP-UX 10.20 here. > This looks like your class LockFile but your code seems quite complex. > I'm not sure that all that is what I needed. My class Lock turned out > to be much simpler I think. If you only want to synchronize your own apps, you do not need the whole complexity from my filelock.rb. But if you wants to work together with the e.g. system mailbox, things are more complicate then. Then you would also have to check if lockfile was stolen and such things. > > That first line is a marvel! "/bin/env ruby" ... hmmm! You do not like it? ;-) I use it anytime! The program 'env' is used for two purposes. 1. Displaying of all environment variable (called without parameter) 2. Start a program with some environment variables set to a certain value *only* for that process. The 2nd point is funny! Suppose you have your DISPLAY variable set to qiao:0, but you will display an xterm on didi:0, you can do this: env DISPLAY=didi:0 xterm -title "Yuhu2" & And now, *only* for the xterm process DISPLAY was set to another value. The nice side effect is, that the program 'env' will search for the executable to be executed in your PATH. So '/bin/env ruby' means: search the program ruby in along PATH. If found start it. So my scripts becomes location independend from the Ruby interpreter. Unfortunately the 'env' programm can be found sometimes in '/bin' sometimes in '/usr/bin'. But there are much more locations possible for Ruby. At home I have a link from '/usr/bin/env' --> '/bin/env'. That makes me really independend! :-))) But now coming to your script... May I propose some changings? Proposing them does not mean your solution is wrong, however! > Here's my code: > > #!/opt/ruby/bin/ruby > > class Lock > attr_accessor :locked_up, :lockdir, :lockfile > > def initialize (lockf = nil) > @locked_up = false > @lockdir = "/var/spool/locks" Although Ruby does not really need it because it handles '/' as separator all the time, I think it would be better to use File::join to do the job: @lockdir = File::Separator + File.join(%w(var spool locks)) > if (lockf == nil) > @lockfile = (@lockdir + "/" + File.basename($0) + ".lock") Here too... !!! @lockfile = File.join(@lockdir, File.basename($0)) + ".lock" > else > @lockfile = (@lockdir + "/" + lockf + ".lock") And here !!! @lockfile = File.join(@lockdir, lockf) + ".lock" > end > end > > def setlock (lockf = @lockfile) > raise "Lockfile not set!" if lockf == nil; > raise "Lock failed!" if > ! system("set -o noclobber ; cat /dev/null 2> /dev/null > #{lockf}") Why do you use such complicate system call here? I would prefer a Ruby solution: if not File.exist?(lockf) begin File.open(lockf, "w") { |fp| fp.write(Process.pid) } rescue raise "Lock failed (system error)" end else raise "Lock failed (already locked)" end Is a little bit longer but portable and deliver two different exceptions depending on the error occured. Okay not different exceptions but different explanations ;-) > > at_exit { self.unlock } > @locked_up = true > end > > def locked? > test(?e, @lockfile) Is ok! But I find the following more self descriptive... !!! File.exist? @lockfile > end > > def Lock.locked? (lockf) > test(?e, "/var/spool/locks/" + lockf) Same as above... !!! File.exist?(File::Separator + File.join(%w(var spool locks)) + lockf > end > > def locked > self.setlock > yield > self.unlock > end > > def unlock > test(?e, @lockfile) if > raise "Unlock failed!" if > ! system("rm -f #{@lockfile}"); What you are try to doing here? If 'rm -f' failed the exception is raised and 'test(?e,...)' will not performed. If 'rm' went well, neither 'raise' nor 'test' will be executed. raise "Unlock failed (no lock)" unless File.exist?(@lockfile) begin File.delete @lockfile rescue raise "Unlock failed (couldn't remove lockfile)" end > > @locked_up = false > end > end > > >>> I use it this way: > > mylock = Lock.new > > mylock.locked { > # ....do stuff.... > } > > >>> or this way: > > return if Lock.locked?("Oracle.lock") > > >>> > > What do you think? Your code is ok! Only there are some things I would like to remark. 1. You often use constants like "/var/spool/locks" throughout your whole code. 2. You use too much system dependend things like system(...) here. This it not really necessary. 3. You should not use things like 'test(?e, ...)'. The Ruby classes are more descriptive. 4. IMHO, your code is not carefully designed for the class/instance clash. Means, you do some things both in class methods and in instance methods. 5. You should not on any case define accessors for your class. I would consider it not-so-good style if I would have to: lock = Lock.new("/var/spool/mail/cle") lock.lockdir = "/var/spool/mail" lock.setlock following would be better, IMO: lock = Lock.new("/var/spool/mail/cle", "/var/spool/mail") lock.setlock All in all I would say, that your code may be appropiate for what you have in mind for your apps. But it is not generic enough, IMHO! No steal-the-lock feature. No wait for the lock. No auto-unlock as soon as I do not need this file instance anymore... But keep fun, ok? :-) \cle -- Clemens Hintze mailto: c.hintze / gmx.net