Skip to content
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 issue 3363: std.stream.readf segfaults with immutable format strings #2224

Merged
merged 3 commits into from Jul 11, 2014

Conversation

Hackerpilot
Copy link
Member

@Hackerpilot Hackerpilot changed the title Fix issue 3363: std.stream.readf segfaults with immutabel format strings Fix issue 3363: std.stream.readf segfaults with immutable format strings Jun 2, 2014
@schveiguy
Copy link
Member

Unit test? Otherwise, I'm going to say LGTM, because it seems ok as a change, and nobody uses din.

@@ -703,7 +703,7 @@ class Stream : InputStream, OutputStream {
}
if (fmt.length == 0 || i == fmt.length) {
i = 0;
if (arguments[j] is typeid(char[])) {
if (arguments[j] is typeid(string) || arguments[j] is typeid(char[])) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about const(char)[]?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed @Hackerpilot ?

@burner
Copy link
Member

burner commented Jul 10, 2014

LGTM

@mihails-strasuns
Copy link

+1 to addition of the unit test

@quickfur
Copy link
Member

Yes, please add unittest, then this looks ready to merge.

@Hackerpilot
Copy link
Member Author

Unit test added.

@quickfur
Copy link
Member

LGTM.

@mihails-strasuns
Copy link

Auto-merge toggled on

mihails-strasuns pushed a commit that referenced this pull request Jul 11, 2014
Fix issue 3363: std.stream.readf segfaults with immutable format strings
@mihails-strasuns mihails-strasuns merged commit 911ea31 into dlang:master Jul 11, 2014
@Hackerpilot Hackerpilot deleted the issue-3363 branch July 12, 2014 00:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
7 participants