Bug #13004: rb_get_kwargs はバグってないでしょうか？ - Ruby master - Ruby Issue Tracking System

Actions

Copy link

Bug #13004

closed

rb_get_kwargs はバグってないでしょうか？

Added by metanest (Makoto Kishimoto) almost 8 years ago. Updated almost 8 years ago.

Status:

Closed

Assignee:

Target version:

ruby -v:

Backport:

2.1: REQUIRED, 2.2: DONE, 2.3: DONE

[ruby-dev:49893]

Description

きしもとです

拡張ライブラリで rb_get_kwargs を使おうとしたところ、いくつか変に
感じた点がありましたので、確認をお願いしたくこちらにメイルします。
もしバグでしたらチケットにまとめます。

(1) valuesを渡すと、全て変更（初期化）される。

extension(.ja).rdoc には、

                       ...  If an optional key is not present in
 +keyword_hash+, the corresponding element in +values+ is not changed.

                                         ..．省略可能キーワー
 ドがない場合は，values中の対応する要素は変更されません．

とあるが、コードでは、

1866     if (values) {
1867         for (j = 0; j < required + optional; j++) {
1868             values[j] = Qundef;
1869         }
1870     }

となっていて、valuesが非NULLであれば一律に Qundef で初期化している。

(2) "unknown keyword" ArgumentError を raise しそこねることがある。

該当部分のコードはこうなっていて、

1898     if (!rest && keyword_hash) {
1899         if (RHASH_SIZE(keyword_hash) > (unsigned int)j) {
1900             unknown_keyword_error(keyword_hash, table, required+optional);
1901         }
1902     }

ここで j には認識されたキーワード引数の個数が入っていて、valuesが
NULLの時にはkeyword_hashの中身が変更されず、恐らくその意図通りの
動作になるが、valuesが非NULLの場合はkeyword_hashからエントリが
除かれるため、認識できなかった引数があってもこのチェックを通り
抜けてしまうことがある（extract_kwargマクロも参照）。

1855 #define extract_kwarg(keyword, val) \
1856     (key = (st_data_t)(keyword), values ? \
1857      st_delete(rb_hash_tbl_raw(keyword_hash), &key, (val)) : \
1858      st_lookup(rb_hash_tbl_raw(keyword_hash), key, (val)))

(3) 使用されない要素はkeyword_hashに残されるだけで、別には保存されない。

extension(.ja).rdoc には、

 If +optional+ is negative, rest of +keyword_hash+ are stored in the
 next to optional +values+ as a new Hash, ...

 keyword_hashに使用されない要素がある場合は，optionalが負なら
 新しいHashとして省略可能引数の次に保存されますが，...

とあるが、該当するコードが見つからない。またもし、そのような動作を
するのであれば、ドキュメントのこの関数に関する記述の冒頭部に、
valuesのサイズは必ず「required+optional + 1」以上でなければ
ならないと書く必要があるように思う。

Actions

Copy link

Updated by nobu (Nobuyoshi Nakada) almost 8 years ago

Status changed from Open to Closed

Applied in changeset r56980.

extension.rdoc: fix rb_get_kwargs [ci skip]

doc/extension.rdoc: [DOC] optional keyword arguments are
defaulted to Qundef. ignored keys are kept in the hash but a
new Hash is not created. [ruby-dev:49893] [Bug #13004]

Actions

Copy link

#2 [ruby-dev:49894]