-
Notifications
You must be signed in to change notification settings - Fork 26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix ascii_only? flag in strio_write #77
Conversation
Wow. |
test/stringio/test_stringio.rb
Outdated
s = StringIO.new('abcd') | ||
s.seek(pos) | ||
s.write("\xff\xff\xff") | ||
refute(s.string.ascii_only?) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
refute(s.string.ascii_only?) | |
assert_operator(s.string, :ascii_only?) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed to refute_predicate(s.string, :ascii_only?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, sorry.
Could you use assert_not_predicate
instead? I don't want to use refute_*
as much as possible because assert_not_
is straightforward for non-native English speaker.
ada1ece
to
acaaa6e
Compare
a051370
to
9faa0eb
Compare
9faa0eb
to
9b9dafa
Compare
I add a test case s = StringIO.new("\u{3042}")
s.rewind
assert_not_predicate(s.string, :ascii_only?)
s.write('aaaa')
assert_predicate(s.string, :ascii_only?) that fails in master (after #79) Writing string in arbitary position(when Other variation: |
Thanks! |
(ruby/stringio#77) Followup of #79 `rb_str_resize()` was changed by b0b9f72 . ```c rb_str_resize(string, shorter) // clear ENC_CODERANGE in some case rb_str_resize(string, longer) // does not clear ENC_CODERANGE anymore ``` ```c // rb_str_resize in string.c if (slen > len && ENC_CODERANGE(str) != ENC_CODERANGE_7BIT) { ENC_CODERANGE_CLEAR(str); } ``` I think this change is based on an assumption that appending null bytes will not change flag `ascii_only?`. `strio_extend()` will make the string longer if needed, and update the flags correctly for appending null bytes. Before `memmove()`, we need to `rb_str_modify()` because updated flags are not updated for `memmove()`. ruby/stringio@b31a538576
Fixes https://bugs.ruby-lang.org/issues/20185Followup of #79
Looks like rb_str_resize is changed in some ruby version
I think this change is based on an assumption that appending null bytes will not change flag
ascii_only?
.strio_extend
will make the string longer if needed, and update the flags correctly for appending null bytes.Before memmove, we need to
rb_str_modify
because updated flags are not updated for memmove.