Skip to content

Commit

Permalink
Remove unnecessary rb_check_type for String (#868)
Browse files Browse the repository at this point in the history 
StringValuePtr() is called after checking the type, but StringValuePtr() also performs type checking.

StringValuePtr() will invoke rb_string_value() which performs Implicit type conversion and type checking.

```
VALUE
rb_string_value(volatile VALUE *ptr)
{
    VALUE s = *ptr;
    if (!RB_TYPE_P(s, T_STRING)) {
	s = rb_str_to_str(s);
	*ptr = s;
    }
    return s;
}
```
  • Loading branch information
@Watson1978
Watson1978 authored Apr 6, 2023
1 parent 8b57d27 commit 2a3b287
Show file tree
Hide file tree
Showing 7 changed files with 10 additions and 35 deletions.
2 changes: 1 addition & 1 deletion ext/oj/dump_leaf.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ inline static void dump_chars(const char *s, size_t size, Out out) {
static void dump_leaf_str(Leaf leaf, Out out) {
switch (leaf->value_type) {
case STR_VAL: oj_dump_cstr(leaf->str, strlen(leaf->str), 0, 0, out); break;
case RUBY_VAL: oj_dump_cstr(rb_string_value_cstr(&leaf->value), (int)RSTRING_LEN(leaf->value), 0, 0, out); break;
case RUBY_VAL: oj_dump_cstr(StringValueCStr(leaf->value), (int)RSTRING_LEN(leaf->value), 0, 0, out); break;
case COL_VAL:
default: rb_raise(rb_eTypeError, "Unexpected value type %02x.\n", leaf->value_type); break;
}
Expand Down
10 changes: 0 additions & 10 deletions ext/oj/fast.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1133,7 +1133,6 @@ static VALUE doc_open_file(VALUE clas, VALUE filename) {
volatile VALUE obj;
int given = rb_block_given_p();

Check_Type(filename, T_STRING);
path = StringValuePtr(filename);
if (0 == (f = fopen(path, "r"))) {
rb_raise(rb_eIOError, "%s", strerror(errno));
Expand Down Expand Up @@ -1305,7 +1304,6 @@ static VALUE doc_type(int argc, VALUE *argv, VALUE self) {
VALUE type = Qnil;

if (1 <= argc) {
Check_Type(*argv, T_STRING);
path = StringValuePtr(*argv);
}
if (0 != (leaf = get_doc_leaf(doc, path))) {
Expand Down Expand Up @@ -1345,7 +1343,6 @@ static VALUE doc_fetch(int argc, VALUE *argv, VALUE self) {

doc = self_doc(self);
if (1 <= argc) {
Check_Type(*argv, T_STRING);
path = StringValuePtr(*argv);
if (2 == argc) {
val = argv[1];
Expand All @@ -1370,7 +1367,6 @@ static VALUE doc_exists(VALUE self, VALUE str) {
Leaf leaf;

doc = self_doc(self);
Check_Type(str, T_STRING);
if (0 != (leaf = get_doc_leaf(doc, StringValuePtr(str)))) {
if (NULL != leaf) {
return Qtrue;
Expand Down Expand Up @@ -1407,7 +1403,6 @@ static VALUE doc_each_leaf(int argc, VALUE *argv, VALUE self) {
memcpy(save_path, doc->where_path, sizeof(Leaf) * (wlen + 1));
}
if (1 <= argc) {
Check_Type(*argv, T_STRING);
path = StringValuePtr(*argv);
if ('/' == *path) {
doc->where = doc->where_path;
Expand Down Expand Up @@ -1442,7 +1437,6 @@ static VALUE doc_move(VALUE self, VALUE str) {
const char *path;
int loc;

Check_Type(str, T_STRING);
path = StringValuePtr(str);
if ('/' == *path) {
doc->where = doc->where_path;
Expand Down Expand Up @@ -1484,7 +1478,6 @@ static VALUE doc_each_child(int argc, VALUE *argv, VALUE self) {
memcpy(save_path, doc->where_path, sizeof(Leaf) * (wlen + 1));
}
if (1 <= argc) {
Check_Type(*argv, T_STRING);
path = StringValuePtr(*argv);
if ('/' == *path) {
doc->where = doc->where_path;
Expand Down Expand Up @@ -1551,7 +1544,6 @@ static VALUE doc_each_value(int argc, VALUE *argv, VALUE self) {
Leaf leaf;

if (1 <= argc) {
Check_Type(*argv, T_STRING);
path = StringValuePtr(*argv);
}
if (0 != (leaf = get_doc_leaf(doc, path))) {
Expand Down Expand Up @@ -1583,11 +1575,9 @@ static VALUE doc_dump(int argc, VALUE *argv, VALUE self) {

if (1 <= argc) {
if (Qnil != *argv) {
Check_Type(*argv, T_STRING);
path = StringValuePtr(*argv);
}
if (2 <= argc) {
Check_Type(argv[1], T_STRING);
filename = StringValuePtr(argv[1]);
}
}
Expand Down
4 changes: 1 addition & 3 deletions ext/oj/oj.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1105,7 +1105,7 @@ static VALUE load_file(int argc, VALUE *argv, VALUE self) {
if (1 > argc) {
rb_raise(rb_eArgError, "Wrong number of arguments to load().");
}
Check_Type(*argv, T_STRING);
path = StringValuePtr(*argv);
parse_info_init(&pi);
pi.options = oj_default_options;
pi.handler = Qnil;
Expand Down Expand Up @@ -1136,7 +1136,6 @@ static VALUE load_file(int argc, VALUE *argv, VALUE self) {
}
}
}
path = StringValuePtr(*argv);
#ifdef _WIN32
{
WCHAR *wide_path;
Expand Down Expand Up @@ -1359,7 +1358,6 @@ static VALUE to_file(int argc, VALUE *argv, VALUE self) {
if (3 == argc) {
oj_parse_options(argv[2], &copts);
}
Check_Type(*argv, T_STRING);
oj_write_obj_to_file(argv[1], StringValuePtr(*argv), &copts);

return Qnil;
Expand Down
13 changes: 6 additions & 7 deletions ext/oj/parser.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1180,7 +1180,7 @@ static int opt_cb(VALUE rkey, VALUE value, VALUE ptr) {
rkey = rb_sym2str(rkey);
// fall through
case RUBY_T_STRING:
key = rb_string_value_ptr(&rkey);
key = StringValuePtr(rkey);
klen = RSTRING_LEN(rkey);
break;
default: rb_raise(rb_eArgError, "option keys must be a symbol or string");
Expand Down Expand Up @@ -1349,7 +1349,7 @@ static VALUE parser_missing(int argc, VALUE *argv, VALUE self) {
case RUBY_T_SYMBOL:
rkey = rb_sym2str(rkey);
// fall through
case RUBY_T_STRING: key = rb_string_value_ptr(&rkey); break;
case RUBY_T_STRING: key = StringValuePtr(rkey); break;
default: rb_raise(rb_eArgError, "option method must be a symbol or string");
}
if (1 < argc) {
Expand All @@ -1366,12 +1366,12 @@ static VALUE parser_missing(int argc, VALUE *argv, VALUE self) {
* Returns the result according to the delegate of the parser.
*/
static VALUE parser_parse(VALUE self, VALUE json) {
ojParser p = (ojParser)DATA_PTR(self);
ojParser p = (ojParser)DATA_PTR(self);
const byte *ptr = (const byte *)StringValuePtr(json);

Check_Type(json, T_STRING);
parser_reset(p);
p->start(p);
parse(p, (const byte *)rb_string_value_ptr(&json));
parse(p, ptr);

return p->result(p);
}
Expand Down Expand Up @@ -1424,8 +1424,7 @@ static VALUE parser_file(VALUE self, VALUE filename) {
const char *path;
int fd;

Check_Type(filename, T_STRING);
path = rb_string_value_ptr(&filename);
path = StringValuePtr(filename);

parser_reset(p);
p->start(p);
Expand Down
4 changes: 2 additions & 2 deletions ext/oj/saj.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -642,7 +642,7 @@ oj_saj_parse(int argc, VALUE *argv, VALUE self) {
s = rb_funcall2(input, oj_string_id, 0, 0);
len = RSTRING_LEN(s) + 1;
json = OJ_R_ALLOC_N(char, len);
strcpy(json, rb_string_value_cstr((VALUE *)&s));
strcpy(json, StringValueCStr(s));
#if !IS_WINDOWS
} else if (rb_cFile == clas && 0 == FIX2INT(rb_funcall(input, oj_pos_id, 0))) {
int fd = FIX2INT(rb_funcall(input, oj_fileno_id, 0));
Expand All @@ -660,7 +660,7 @@ oj_saj_parse(int argc, VALUE *argv, VALUE self) {
s = rb_funcall2(input, oj_read_id, 0, 0);
len = RSTRING_LEN(s) + 1;
json = OJ_R_ALLOC_N(char, len);
strcpy(json, rb_string_value_cstr((VALUE *)&s));
strcpy(json, StringValueCStr(s));
} else {
rb_raise(rb_eArgError, "saj_parse() expected a String or IO Object.");
}
Expand Down
6 changes: 0 additions & 6 deletions ext/oj/stream_writer.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,6 @@ static VALUE stream_writer_new(int argc, VALUE *argv, VALUE self) {
static VALUE stream_writer_push_key(VALUE self, VALUE key) {
StreamWriter sw = (StreamWriter)DATA_PTR(self);

rb_check_type(key, T_STRING);
oj_str_writer_push_key(&sw->sw, StringValuePtr(key));
if (sw->flush_limit < sw->sw.out.cur - sw->sw.out.buf) {
stream_writer_write(sw);
Expand All @@ -160,7 +159,6 @@ static VALUE stream_writer_push_object(int argc, VALUE *argv, VALUE self) {
if (Qnil == argv[0]) {
oj_str_writer_push_object(&sw->sw, 0);
} else {
rb_check_type(argv[0], T_STRING);
oj_str_writer_push_object(&sw->sw, StringValuePtr(argv[0]));
}
break;
Expand Down Expand Up @@ -189,7 +187,6 @@ static VALUE stream_writer_push_array(int argc, VALUE *argv, VALUE self) {
if (Qnil == argv[0]) {
oj_str_writer_push_array(&sw->sw, 0);
} else {
rb_check_type(argv[0], T_STRING);
oj_str_writer_push_array(&sw->sw, StringValuePtr(argv[0]));
}
break;
Expand Down Expand Up @@ -217,7 +214,6 @@ static VALUE stream_writer_push_value(int argc, VALUE *argv, VALUE self) {
if (Qnil == argv[1]) {
oj_str_writer_push_value((StrWriter)DATA_PTR(self), *argv, 0);
} else {
rb_check_type(argv[1], T_STRING);
oj_str_writer_push_value((StrWriter)DATA_PTR(self), *argv, StringValuePtr(argv[1]));
}
break;
Expand All @@ -241,14 +237,12 @@ static VALUE stream_writer_push_value(int argc, VALUE *argv, VALUE self) {
static VALUE stream_writer_push_json(int argc, VALUE *argv, VALUE self) {
StreamWriter sw = (StreamWriter)DATA_PTR(self);

rb_check_type(argv[0], T_STRING);
switch (argc) {
case 1: oj_str_writer_push_json((StrWriter)DATA_PTR(self), StringValuePtr(*argv), 0); break;
case 2:
if (Qnil == argv[1]) {
oj_str_writer_push_json((StrWriter)DATA_PTR(self), StringValuePtr(*argv), 0);
} else {
rb_check_type(argv[1], T_STRING);
oj_str_writer_push_json((StrWriter)DATA_PTR(self), StringValuePtr(*argv), StringValuePtr(argv[1]));
}
break;
Expand Down
6 changes: 0 additions & 6 deletions ext/oj/string_writer.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,6 @@ static VALUE str_writer_new(int argc, VALUE *argv, VALUE self) {
static VALUE str_writer_push_key(VALUE self, VALUE key) {
StrWriter sw = (StrWriter)DATA_PTR(self);

rb_check_type(key, T_STRING);
oj_str_writer_push_key(sw, StringValuePtr(key));

return Qnil;
Expand All @@ -303,7 +302,6 @@ static VALUE str_writer_push_object(int argc, VALUE *argv, VALUE self) {
if (Qnil == argv[0]) {
oj_str_writer_push_object(sw, 0);
} else {
rb_check_type(argv[0], T_STRING);
oj_str_writer_push_object(sw, StringValuePtr(argv[0]));
}
break;
Expand Down Expand Up @@ -332,7 +330,6 @@ static VALUE str_writer_push_array(int argc, VALUE *argv, VALUE self) {
if (Qnil == argv[0]) {
oj_str_writer_push_array(sw, 0);
} else {
rb_check_type(argv[0], T_STRING);
oj_str_writer_push_array(sw, StringValuePtr(argv[0]));
}
break;
Expand All @@ -359,7 +356,6 @@ static VALUE str_writer_push_value(int argc, VALUE *argv, VALUE self) {
if (Qnil == argv[1]) {
oj_str_writer_push_value((StrWriter)DATA_PTR(self), *argv, 0);
} else {
rb_check_type(argv[1], T_STRING);
oj_str_writer_push_value((StrWriter)DATA_PTR(self), *argv, StringValuePtr(argv[1]));
}
break;
Expand All @@ -378,14 +374,12 @@ static VALUE str_writer_push_value(int argc, VALUE *argv, VALUE self) {
* - *key* [_String_] the key if adding to an object in the JSON document
*/
static VALUE str_writer_push_json(int argc, VALUE *argv, VALUE self) {
rb_check_type(argv[0], T_STRING);
switch (argc) {
case 1: oj_str_writer_push_json((StrWriter)DATA_PTR(self), StringValuePtr(*argv), 0); break;
case 2:
if (Qnil == argv[1]) {
oj_str_writer_push_json((StrWriter)DATA_PTR(self), StringValuePtr(*argv), 0);
} else {
rb_check_type(argv[1], T_STRING);
oj_str_writer_push_json((StrWriter)DATA_PTR(self), StringValuePtr(*argv), StringValuePtr(argv[1]));
}
break;
Expand Down

0 comments on commit 2a3b287

Please sign in to comment.