* ext/syslog/syslog.c: Cut redundancy.

* ext/syslog/syslog.c: Do not leak ident.

* ext/syslog/syslog.c, ext/syslog/test.rb: Syslog.close should
  raise RuntimeError when not opened.

* ext/syslog/syslog.c, ext/syslog/test.rb:
  Syslog.{ident,options,facility,mask} should all return nil when
  not opened.

* ext/syslog/syslog.c, ext/syslog/test.rb: Change back the output
  format of inspect().


git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/branches/ruby_1_6@3095 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

2 files changed, 59 insertions, 31 deletions

diff --git a/ext/syslog/syslog.c b/ext/syslog/syslog.c
index 98fb5ae4f3..3e86569f06 100644
--- a/ext/syslog/syslog.c
+++ b/ext/syslog/syslog.c
@@ -37,7 +37,15 @@ static void syslog_write(int pri, int argc, VALUE *argv)
 /* Syslog module methods */
 static VALUE mSyslog_close(VALUE self)
 {
+    if (!syslog_opened) {
+        rb_raise(rb_eRuntimeError, "syslog not opened");
+    }
+
     closelog();
+
+    free((void *)syslog_ident);
+    syslog_ident = NULL;
+    syslog_options = syslog_facility = syslog_mask = -1;
     syslog_opened = 0;
 
     return Qnil;
@@ -51,26 +59,33 @@ static VALUE mSyslog_open(int argc, VALUE *argv, VALUE self)
     if (syslog_opened) {
         rb_raise(rb_eRuntimeError, "syslog already open");
     }
+
     rb_scan_args(argc, argv, "03", &ident, &opt, &fac);
+
     if (NIL_P(ident)) {
         ident = rb_gv_get("$0"); 
     }
-    if (NIL_P(opt)) {
-        opt = INT2NUM(LOG_PID | LOG_CONS);
-    }
-    if (NIL_P(fac)) {
-        fac = INT2NUM(LOG_USER);
-    }
-
 #ifdef SafeStringValue
     SafeStringValue(ident);
 #else
     Check_SafeStr(ident);
 #endif
-    syslog_ident = (const char *)strdup(RSTRING(ident)->ptr);
-    syslog_options = NUM2INT(opt);
-    syslog_facility = NUM2INT(fac);
+    syslog_ident = strdup(RSTRING(ident)->ptr);
+
+    if (NIL_P(opt)) {
+	syslog_options = LOG_PID | LOG_CONS;
+    } else {
+	syslog_options = NUM2INT(opt);
+    }
+
+    if (NIL_P(fac)) {
+	syslog_facility = LOG_USER;
+    } else {
+	syslog_facility = NUM2INT(fac);
+    }
+
     openlog(syslog_ident, syslog_options, syslog_facility);
+
     syslog_opened = 1;
 
     setlogmask(syslog_mask = setlogmask(0));
@@ -97,22 +112,22 @@ static VALUE mSyslog_isopen(VALUE self)
 
 static VALUE mSyslog_ident(VALUE self)
 {
-    return rb_str_new2(syslog_ident);
+    return syslog_opened ? rb_str_new2(syslog_ident) : Qnil;
 }
 
 static VALUE mSyslog_options(VALUE self)
 {
-    return INT2NUM(syslog_options);
+    return syslog_opened ? INT2NUM(syslog_options) : Qnil;
 }
 
 static VALUE mSyslog_facility(VALUE self)
 {
-    return INT2NUM(syslog_facility);
+    return syslog_opened ? INT2NUM(syslog_facility) : Qnil;
 }
 
 static VALUE mSyslog_get_mask(VALUE self)
 {
-    return INT2NUM(syslog_mask);
+    return syslog_opened ? INT2NUM(syslog_mask) : Qnil;
 }
 
 static VALUE mSyslog_set_mask(VALUE self, VALUE mask)
@@ -150,14 +165,18 @@ static VALUE mSyslog_inspect(VALUE self)
 {
     char buf[1024];
 
-    snprintf(buf, sizeof(buf),
-      "<#%s: ident=\"%s\", options=%d, facility=%d, mask=%d%s>",
-      rb_class2name(self),
-      syslog_ident,
-      syslog_options,
-      syslog_facility,
-      syslog_mask,
-      syslog_opened ? ", opened" : "");
+    if (syslog_opened) {
+	snprintf(buf, sizeof(buf),
+	  "<#%s: opened=true, ident=\"%s\", options=%d, facility=%d, mask=%d>",
+	  rb_class2name(self),
+	  syslog_ident,
+	  syslog_options,
+	  syslog_facility,
+	  syslog_mask);
+    } else {
+	snprintf(buf, sizeof(buf),
+	  "<#%s: opened=false>", rb_class2name(self));
+    }
 
     return rb_str_new2(buf);
 }
diff --git a/ext/syslog/test.rb b/ext/syslog/test.rb
index 4b25852c36..5df769dbfb 100644
--- a/ext/syslog/test.rb
+++ b/ext/syslog/test.rb
@@ -28,7 +28,7 @@ class TestSyslog < Test::Unit::TestCase
     assert_equal(Syslog, sl2)
     assert_equal(Syslog, sl3)
   ensure
-    Syslog.close
+    Syslog.close if Syslog.opened?
   end
 
   def test_open
@@ -59,16 +59,16 @@ class TestSyslog < Test::Unit::TestCase
     Syslog.open
     Syslog.close
 
-    assert_equal($0, Syslog.ident)
-    assert_equal(Syslog::LOG_PID | Syslog::LOG_CONS, Syslog.options)
-    assert_equal(Syslog::LOG_USER, Syslog.facility)
+    assert_equal(nil, Syslog.ident)
+    assert_equal(nil, Syslog.options)
+    assert_equal(nil, Syslog.facility)
 
     # block
     param = nil
     Syslog.open { |param| }
     assert_equal(Syslog, param)
   ensure
-    Syslog.close
+    Syslog.close if Syslog.opened?
   end
 
   def test_opened?
@@ -87,7 +87,15 @@ class TestSyslog < Test::Unit::TestCase
     assert_equal(false, Syslog.opened?)
   end
 
+  def test_close
+    assert_raises(RuntimeError) {
+      Syslog.close
+    }
+  end
+
   def test_mask
+    assert_equal(nil, Syslog.mask)
+
     Syslog.open
 
     orig = Syslog.mask
@@ -100,7 +108,7 @@ class TestSyslog < Test::Unit::TestCase
 
     Syslog.mask = orig
   ensure
-    Syslog.close
+    Syslog.close if Syslog.opened?
   end
 
   def test_log
@@ -142,14 +150,15 @@ class TestSyslog < Test::Unit::TestCase
 
   def test_inspect
     Syslog.open { |sl|
-      assert_equal(format('<#%s: ident="%s", options=%d, facility=%d, mask=%d%s>',
+      assert_equal(format('<#%s: opened=true, ident="%s", options=%d, facility=%d, mask=%d>',
 			  Syslog,
 			  sl.ident,
 			  sl.options,
 			  sl.facility,
-			  sl.mask,
-			  sl.opened? ? ', opened' : ''),
+			  sl.mask),
 		   sl.inspect)
     }
+
+    assert_equal(format('<#%s: opened=false>', Syslog), Syslog.inspect)
   end
 end