Skip to content

fix unsafe .ptr access in Phobos#4426

Merged
schveiguy merged 1 commit intodlang:masterfrom
WalterBright:fixUnsafePtr
Jun 14, 2016
Merged

fix unsafe .ptr access in Phobos#4426
schveiguy merged 1 commit intodlang:masterfrom
WalterBright:fixUnsafePtr

Conversation

@WalterBright
Copy link
Member

Necessary for dlang/dmd#5860


return retval.ptr;
auto trustedPtr() @trusted { return retval.ptr; }
return trustedPtr();
Copy link
Member

Choose a reason for hiding this comment

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

Even though this LGTM, creating a function that can be called any time is more dangerous than an anonymous lambda that is called once:

return (() @trusted => retval.ptr)();

And it's shorter code too.

Copy link
Member

Choose a reason for hiding this comment

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

It's a local function anyhow so the damage is narrowed down.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not saying it's awful and should cause the PR not to be pulled. What I'm saying is that it seems unnecessarily open for abuse. I could see a refactor at some point somehow open it up to the rest of the function.

It's also more verbose. If we can do better, shouldn't we?

Copy link
Contributor

@dnadlinger dnadlinger Jun 13, 2016

Choose a reason for hiding this comment

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

@ntrel: Isn't this just "escaping" a pointer to a literal (GC-allocated if P is mutable, global data otherwise)?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, sorry.

Copy link
Member

Choose a reason for hiding this comment

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

global data otherwise

oooh, is that new? I always thought [element] would allocate, even if element is immutable.

@schveiguy
Copy link
Member

I'll pull this, the difference between a trusted lambda and a separate function is pretty small, especially in a small block.

@schveiguy
Copy link
Member

Auto-merge toggled on

@schveiguy schveiguy merged commit 9c0d3f4 into dlang:master Jun 14, 2016
@WalterBright WalterBright deleted the fixUnsafePtr branch June 15, 2016 00:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants