[ruby-core:68511] Re: [Ruby trunk - Feature #10532] [PATCH] accept_nonblock supports "exception: false"

Nobuyoshi Nakada <[e-mail]> wrote:
> On 2015/03/12 17:38, Nobuyoshi Nakada wrote:
> > On 2015/03/12 12:08, Eric Wong wrote:
> >> I think the st_delete call in rb_get_kwargs interacts badly with with
> >> keyword optimizations ko1 did in 2.2.  My work-in-progress patch
> >> gets stuck on the OpenSSL read_nonblock (from
> >> ext/openssl/lib/openssl/buffering.rb) in test/openssl/test_ssl.rb
> > 
> > It is an intentional behavior.
> > 
> > Add this after `rb_get_kwargs()` call.
> > 
> >     if (except != Qundef)
> > 	rb_hash_aset(opts, ID2SYM(id_exception), except);

That seems tricky.  We should to document it, at least:

diff --git a/README.EXT b/README.EXT
index f24029c..f87ad85 100644
--- a/README.EXT
+++ b/README.EXT
@@ -1308,7 +1308,8 @@ rb_scan_args(int argc, VALUE *argv, const char *fmt, ...) ::
 int rb_get_kwargs(VALUE keyword_hash, const ID *table, int required, int optional, VALUE *values)
 
   Retrieves argument VALUEs bound to keywords, which directed by +table+
-  into +values+.  First +required+ number of IDs referred by +table+ are
+  into +values+, deleting retrieved entries from +keyword_hash+ along
+  the way.  First +required+ number of IDs referred by +table+ are
   mandatory, and succeeding +optional+ (- +optional+ - 1 if
   +optional+ is negative) number of IDs are optional.  If a
   mandatory key is not contained in +keyword_hash+, raises "missing
@@ -1316,7 +1317,8 @@ int rb_get_kwargs(VALUE keyword_hash, const ID *table, int required, int optiona
   +keyword_hash+, the corresponding element in +values+ is not changed.
   If +optional+ is negative, rest of +keyword_hash+ are stored in the
   next to optional +values+ as a new Hash, otherwise raises "unknown
-  keyword" +ArgumentError+.
+  keyword" +ArgumentError+.  If +values+ is NULL, +keyword_hash+ is
+  not modified and only error checking is performed.
 
 VALUE rb_extract_keywords(VALUE *original_hash)
 

I'll commit the above unless somebody else has better ideas.

> Or you can just `rb_hash_lookup2(opts, ID2SYM(id_exception), Qundef)`,
> if you want to pass the whole options without checking.

Alright, I'll do that (not much different from current rb_hash_aref).