clogger RubyGem user+dev discussion/patches/pulls/bugs/help
 help / Atom feed
* [RFC/PATCH] remove :to_io support
@ 2013-12-16 20:41 Eric Wong
  0 siblings, 0 replies; 1+ messages in thread
From: Eric Wong @ 2013-12-16 20:41 UTC (permalink / raw)
  To: clogger

:to_io never was a Rack extension, and ends up breaking the case
where an SSL socket is proxied.  The role of :to_io in IO-like
objects is to aid IO.select and like methods.
---
 ext/clogger_ext/clogger.c         | 46 +++++++++------------------------------
 ext/clogger_ext/ruby_1_9_compat.h | 31 --------------------------
 lib/clogger/pure.rb               |  9 +-------
 test/test_clogger_to_path.rb      | 37 -------------------------------
 4 files changed, 11 insertions(+), 112 deletions(-)

diff --git a/ext/clogger_ext/clogger.c b/ext/clogger_ext/clogger.c
index 8995932..646af62 100644
--- a/ext/clogger_ext/clogger.c
+++ b/ext/clogger_ext/clogger.c
@@ -130,7 +130,6 @@ static ID size_id;
 static ID sq_brace_id;
 static ID new_id;
 static ID to_path_id;
-static ID to_io_id;
 static ID respond_to_id;
 static VALUE cClogger;
 static VALUE mFormat;
@@ -988,22 +987,17 @@ static VALUE to_path(VALUE self)
 	struct stat sb;
 	int rv;
 	VALUE path = rb_funcall(c->body, to_path_id, 0);
+	const char *cpath = StringValueCStr(path);
+	unsigned devfd;
 
-	/* try to avoid an extra path lookup  */
-	if (rb_respond_to(c->body, to_io_id)) {
-		rv = fstat(my_fileno(c->body), &sb);
-	} else {
-		const char *cpath = StringValueCStr(path);
-		unsigned devfd;
-		/*
-		 * Rainbows! can use "/dev/fd/%u" in to_path output to avoid
-		 * extra open() syscalls, too.
-		 */
-		if (sscanf(cpath, "/dev/fd/%u", &devfd) == 1)
-			rv = fstat((int)devfd, &sb);
-		else
-			rv = stat(cpath, &sb);
-	}
+	/*
+	 * Rainbows! can use "/dev/fd/%u" in to_path output to avoid
+	 * extra open() syscalls, too.
+	 */
+	if (sscanf(cpath, "/dev/fd/%u", &devfd) == 1)
+		rv = fstat((int)devfd, &sb);
+	else
+		rv = stat(cpath, &sb);
 
 	/*
 	 * calling this method implies the web server will bypass
@@ -1014,24 +1008,6 @@ static VALUE to_path(VALUE self)
 	return path;
 }
 
-/*
- * call-seq:
- *   clogger.to_io
- *
- * used to proxy +:to_io+ method calls to the wrapped response body.
- */
-static VALUE to_io(VALUE self)
-{
-	struct clogger *c = clogger_get(self);
-	struct stat sb;
-	VALUE io = rb_convert_type(c->body, T_FILE, "IO", "to_io");
-
-	if (fstat(my_fileno(io), &sb) == 0)
-		c->body_bytes_sent = sb.st_size;
-
-	return io;
-}
-
 /* :nodoc: */
 static VALUE body(VALUE self)
 {
@@ -1055,7 +1031,6 @@ void Init_clogger_ext(void)
 	sq_brace_id = rb_intern("[]");
 	new_id = rb_intern("new");
 	to_path_id = rb_intern("to_path");
-	to_io_id = rb_intern("to_io");
 	respond_to_id = rb_intern("respond_to?");
 	cClogger = rb_define_class("Clogger", rb_cObject);
 	mFormat = rb_define_module_under(cClogger, "Format");
@@ -1069,7 +1044,6 @@ void Init_clogger_ext(void)
 	rb_define_method(cClogger, "wrap_body?", clogger_wrap_body, 0);
 	rb_define_method(cClogger, "reentrant?", clogger_reentrant, 0);
 	rb_define_method(cClogger, "to_path", to_path, 0);
-	rb_define_method(cClogger, "to_io", to_io, 0);
 	rb_define_method(cClogger, "respond_to?", respond_to, 1);
 	rb_define_method(cClogger, "body", body, 0);
 	CONST_GLOBAL_STR(REMOTE_ADDR);
diff --git a/ext/clogger_ext/ruby_1_9_compat.h b/ext/clogger_ext/ruby_1_9_compat.h
index 30a38d6..e0ba4ac 100644
--- a/ext/clogger_ext/ruby_1_9_compat.h
+++ b/ext/clogger_ext/ruby_1_9_compat.h
@@ -21,34 +21,3 @@ static void rb_18_str_set_len(VALUE str, long len)
 }
 #define rb_str_set_len(str,len) rb_18_str_set_len(str,len)
 #endif
-
-#if ! HAVE_RB_IO_T
-#  define rb_io_t OpenFile
-#endif
-
-#ifdef GetReadFile
-#  define FPTR_TO_FD(fptr) (fileno(GetReadFile(fptr)))
-#else
-#  if !HAVE_RB_IO_T || (RUBY_VERSION_MAJOR == 1 && RUBY_VERSION_MINOR == 8)
-#    define FPTR_TO_FD(fptr) fileno(fptr->f)
-#  else
-#    define FPTR_TO_FD(fptr) fptr->fd
-#  endif
-#endif
-
-static int my_fileno(VALUE io)
-{
-	rb_io_t *fptr;
-
-	for (;;) {
-		switch (TYPE(io)) {
-		case T_FILE: {
-			GetOpenFile(io, fptr);
-			return FPTR_TO_FD(fptr);
-		}
-		default:
-			io = rb_convert_type(io, T_FILE, "IO", "to_io");
-			/* retry */
-		}
-	}
-}
diff --git a/lib/clogger/pure.rb b/lib/clogger/pure.rb
index 44f4e62..bb3fc16 100644
--- a/lib/clogger/pure.rb
+++ b/lib/clogger/pure.rb
@@ -82,17 +82,10 @@ class Clogger
 
   def to_path
     rv = @body.to_path
-    # try to avoid unnecessary path lookups with to_io.stat instead of
-    # File.size
-    @body_bytes_sent =
-           @body.respond_to?(:to_io) ? @body.to_io.stat.size : File.size(rv)
+    @body_bytes_sent = File.size(rv)
     rv
   end
 
-  def to_io
-    @body.to_io
-  end
-
 private
 
   def byte_xs(s)
diff --git a/test/test_clogger_to_path.rb b/test/test_clogger_to_path.rb
index 4cc9027..b437695 100644
--- a/test/test_clogger_to_path.rb
+++ b/test/test_clogger_to_path.rb
@@ -93,43 +93,6 @@ class TestCloggerToPath < Test::Unit::TestCase
     assert_equal "365 200\n", logger.string
   end
 
-  def test_wraps_to_path_to_io
-    logger = StringIO.new
-    tmp = Tempfile.new('')
-    def tmp.to_io
-      @to_io_called = super
-    end
-    def tmp.to_path
-      path
-    end
-    app = Rack::Builder.new do
-      tmp.syswrite(' ' * 365)
-      tmp.sysseek(0)
-      h = {
-        'Content-Length' => '0',
-        'Content-Type' => 'text/plain',
-      }
-      use Clogger,
-        :logger => logger,
-        :reentrant => true,
-        :format => '$body_bytes_sent $status'
-      run lambda { |env| [ 200, h, tmp ] }
-    end.to_app
-
-    status, headers, body = app.call(@req)
-    assert_instance_of(Clogger, body)
-    check_body(body)
-
-    assert_equal tmp.path, body.to_path
-    assert_nothing_raised { body.to_io }
-    assert_kind_of IO, tmp.instance_variable_get(:@to_io_called)
-    assert logger.string.empty?
-    assert ! tmp.closed?
-    body.close
-    assert tmp.closed?
-    assert_equal "365 200\n", logger.string
-  end
-
   def test_does_not_wrap_to_path
     logger = StringIO.new
     app = Rack::Builder.new do
-- 
Eric Wong


^ permalink raw reply	[flat|nested] 1+ messages in thread

only message in thread, back to index

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-12-16 20:41 [RFC/PATCH] remove :to_io support Eric Wong

clogger RubyGem user+dev discussion/patches/pulls/bugs/help

Archives are clonable:
	git clone --mirror https://bogomips.org/clogger-public
	git clone --mirror http://ou63pmih66umazou.onion/clogger-public

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.lang.ruby.clogger
	nntp://ou63pmih66umazou.onion/inbox.comp.lang.ruby.clogger

 note: .onion URLs require Tor: https://www.torproject.org/
       or Tor2web: https://www.tor2web.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox