Bug 8471 – std.stdio.readf should be @trusted

http://dlang.org/phobos/std_stdio.html#readf why readf do not use reference ? as : uint readf(Data...)(in char[] format, ref Data data); it is not possible? I think this little thing could enhance to have a D cohesive syntax. Of course D support pointer but ref is more in D spirit and point C spirit. They are not many phobos function where need a pointer or an address. Thanks, i hope my talk is not stupid kind regards

When readf was defined, ref didn't work with variadics. Closing because fixing behavior now would break existing code.

(In reply to comment #1) > When readf was defined, ref didn't work with variadics. Closing because fixing > behavior now would break existing code. Maybe a less bug prone and different named function should be added, that uses ref...

Actually it's not that bad - readf is not bug prone because it statically ensures that all of its parameters are pointers. Using pointers is also safe because readf doesn't escape them. Actually I'm reopening this with a different title.

According to Steven's blog post (http://dlang.org/blog/2016/09/28/how-to-write-trusted-code-in-d/) I should "never use @trusted on template functions that accept arbitrary types". Is the readf case special in this regard?

(In reply to Jakub Łabaj from comment #4) > According to Steven's blog post > (http://dlang.org/blog/2016/09/28/how-to-write-trusted-code-in-d/) I should > "never use @trusted on template functions that accept arbitrary types". Is > the readf case special in this regard? If readf calls user-defined functions (constructor, assignment) then yes that's a problem. The smoking gun would be an unsafe unittest that passes with the current implementation. Can you write one?

Currently I see one way to break the safety, which is to not pass a real pointer, but a structure with unary '*' overloaded: @safe unittest { struct Unsafe { int* x; ref int opUnary(string s)() if (s == "*") { int y; // int* ptr = &y; // not @safe return *x; } } static int x; static Unsafe unsafe; unsafe.x = &x; string text = "10"; formattedRead(text, "%d ", unsafe); // called by readf assert(*unsafe.x == 10); } Probably I can't mess up assignment operator nor constructor, because only builtin types are parsable (constrained by function unformatValue). So I think making formattedRead / readf accepting only pointers to builtin types is a way to make them @trusted.

Cool, thanks, then the bug is legit. The fix would be a @safe function with a small @trusted core.

Sorry, I'm not sure what you mean by that - what are the next steps to do here?

(In reply to Jakub Łabaj from comment #8) > Sorry, I'm not sure what you mean by that - what are the next steps to do > here? I think Razvan Nitu has reached out to you on how to go about creating PRs.

I know how to create PRs, I've already created some. What I mean is I'm not sure how you see the solution, e.g. '@safe function with a small @trusted core', could elaborate on this, please?

Oh, sorry. The idea is to leave readf unqualified and let the compiler infer whether it's safe or not. In this particular case I see there's a simple solution - just add a constraint to it making sure all parameters are pointers. Something like: uint readf(Data...)(in char[] format, Data data) if (allSatisfy!(isPointer, Data); Then the only way to call readf is with pointers, which eliminates the possibility of shenanigans.

@Jakub, what's your github id? thx!

I understand now, thanks! You can find my profile here: https://github.com/byebye. I've create a simple PR: https://github.com/dlang/phobos/pull/5040 for similar issue involving formattedRead, where I was wondering if I should restrict arguments to pointers. So it seems the solution would be exactly the same in both cases.

Commits pushed to master at https://github.com/dlang/phobos https://github.com/dlang/phobos/commit/882a1fb8f62b872f3463e9fca2b2e1e04d36effb Fix issue #8471 - allow only pointers as readf parameters https://github.com/dlang/phobos/commit/0059fc3263b9ebd62ef7c6c7b3eed4b20f313364 Merge pull request #5076 from byebye/issue_8471 Fix issue #8471 - allow only pointers as readf parameters

It's not fixed yet, my commit doesn't really make readf @safe/@trusted (sorry for the mess).

Commits pushed to newCTFE at https://github.com/dlang/phobos https://github.com/dlang/phobos/commit/882a1fb8f62b872f3463e9fca2b2e1e04d36effb Fix issue #8471 - allow only pointers as readf parameters https://github.com/dlang/phobos/commit/0059fc3263b9ebd62ef7c6c7b3eed4b20f313364 Merge pull request #5076 from byebye/issue_8471

(wrongly closed)

As Andrei pointed out, readf has its signature for historic reasons. Please check out my pull: https://github.com/dlang/phobos/pull/5247 If I'm correct, we can deprecate formattedRead and with it readf with pointers. Tell me if I'm wrong, but there is no justification to make readf @trusted. It accesses a __gshared global and is not @safe for good reasons.

@Bolpat (In reply to Bolpat from comment #18) > As Andrei pointed out, readf has its signature for historic reasons. Please > check out my pull: https://github.com/dlang/phobos/pull/5247 If I'm correct, > we can deprecate formattedRead and with it readf with pointers. > > Tell me if I'm wrong, but there is no justification to make readf @trusted. > It accesses a __gshared global and is not @safe for good reasons. Using __gshared is not problematic because the reading primitives are interlocked.

Commits pushed to stable at https://github.com/dlang/phobos https://github.com/dlang/phobos/commit/882a1fb8f62b872f3463e9fca2b2e1e04d36effb Fix issue #8471 - allow only pointers as readf parameters https://github.com/dlang/phobos/commit/0059fc3263b9ebd62ef7c6c7b3eed4b20f313364 Merge pull request #5076 from byebye/issue_8471