Skip to content

rangify readLink and symlink #3967

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.

Sign up for GitHub

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

Merged
DmitryOlshansky merged 3 commits into from
Apr 26, 2016
Merged

rangify readLink and symlink #3967

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dce9175
rangify readLink and symlink
aG0aep6G Feb 3, 2016
831ddcc
code formattting
aG0aep6G Feb 5, 2016
863b251
smoothen out differences between readLink and symlink
aG0aep6G Feb 5, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions changelog.dd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ $(BUGSTITLE Library Changes,
of the type being iterated)
$(LI $(XREF algorithm, sorting, isStrictlyMonotonic) which doesn't allow
equal values was added.)
$(LI $(REF readLink, std,file) and $(REF symlink, std,file) have been
rangified.)
)

$(BUGSTITLE Library Changes,
Expand Down
128 changes: 91 additions & 37 deletions std/file.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2310,15 +2310,31 @@ unittest
$(D FileException) on error (which includes if the _symlink already
exists).
+/
version(StdDdoc) void symlink(C1, C2)(const(C1)[] original, const(C2)[] link) @safe;
else version(Posix) void symlink(C1, C2)(const(C1)[] original, const(C2)[] link) @safe
{
static auto trustedSymlink(const(C1)[] path1, const(C2)[] path2) @trusted
version(StdDdoc) void symlink(RO, RL)(RO original, RL link)
if ((isInputRange!RO && isSomeChar!(ElementEncodingType!RO) ||
isConvertibleToString!RO) &&
(isInputRange!RL && isSomeChar!(ElementEncodingType!RL) ||
isConvertibleToString!RL));
else version(Posix) void symlink(RO, RL)(RO original, RL link)
if ((isInputRange!RO && isSomeChar!(ElementEncodingType!RO) ||
Copy link
Contributor

@JakobOvrum JakobOvrum Feb 5, 2016

Choose a reason for hiding this comment

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

It made sense that the calls to tempCString were within the @trusted block before, despite tempCString itself being mistakenly @trusted. But I guess the unit tests would ensure that this is properly rectified if we decide to fix tempCString.

Copy link
Contributor Author

@aG0aep6G aG0aep6G Feb 5, 2016

Choose a reason for hiding this comment

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

The tempCString calls must not be @trusted. The types on which tempCString is called were more restricted before: only arrays, i.e. no user code. Now it's called on unverified, user-supplied ranges. Calling it in an @trusted block would make unverified code @trusted.

Copy link
Contributor

@JakobOvrum JakobOvrum Feb 6, 2016

Choose a reason for hiding this comment

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

Right, of course.

isConvertibleToString!RO) &&
(isInputRange!RL && isSomeChar!(ElementEncodingType!RL) ||
isConvertibleToString!RL))
{
static if (isConvertibleToString!RO || isConvertibleToString!RL)
{
import std.meta : staticMap;
alias Types = staticMap!(convertToString, RO, RL);
symlink!Types(original, link);
}
else
{
return core.sys.posix.unistd.symlink(path1.tempCString(),
path2.tempCString());
auto oz = original.tempCString();
auto lz = link.tempCString();
alias posixSymlink = core.sys.posix.unistd.symlink;
immutable int result = () @trusted { return posixSymlink(oz, lz); } ();
cenforce(result == 0, text(link));
}
cenforce(trustedSymlink(original, link) == 0, link);
}

version(Posix) @safe unittest
Expand Down Expand Up @@ -2364,6 +2380,12 @@ version(Posix) @safe unittest
}
}

version(Posix) unittest
{
static assert(__traits(compiles,
symlink(TestAliasedString(null), TestAliasedString(null))));
}


/++
$(BLUE This function is Posix-Only.)
Expand All @@ -2376,44 +2398,55 @@ version(Posix) @safe unittest
Throws:
$(D FileException) on error.
+/
version(StdDdoc) string readLink(C)(const(C)[] link) @safe;
else version(Posix) string readLink(C)(const(C)[] link) @safe
{
static auto trustedReadlink(const(C)[] path, char[] buf) @trusted
version(StdDdoc) string readLink(R)(R link)
if (isInputRange!R && isSomeChar!(ElementEncodingType!R) ||
isConvertibleToString!R);
else version(Posix) string readLink(R)(R link)
if (isInputRange!R && isSomeChar!(ElementEncodingType!R) ||
isConvertibleToString!R)
{
static if (isConvertibleToString!R)
{
return core.sys.posix.unistd.readlink(path.tempCString(), buf.ptr, buf.length);
return readLink!(convertToString!R)(link);
}
static auto trustedAssumeUnique(ref C[] array) @trusted
else
{
return assumeUnique(array);
}

enum bufferLen = 2048;
enum maxCodeUnits = 6;
char[bufferLen] buffer;
auto size = trustedReadlink(link, buffer);
cenforce(size != -1, link);

if(size <= bufferLen - maxCodeUnits)
return to!string(buffer[0 .. size]);

auto dynamicBuffer = new char[](bufferLen * 3 / 2);
alias posixReadlink = core.sys.posix.unistd.readlink;
enum bufferLen = 2048;
enum maxCodeUnits = 6;
char[bufferLen] buffer;
Copy link
Member

@DmitryOlshansky DmitryOlshansky Apr 26, 2016

Choose a reason for hiding this comment

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

How about a test to cover at least > 2048 path case?

Copy link
Contributor Author

@aG0aep6G aG0aep6G Apr 26, 2016

Choose a reason for hiding this comment

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

I didn't write this, just indented it. Let's fix pre-existing issues separately.

const linkz = link.tempCString();
auto size = () @trusted {
return posixReadlink(linkz, buffer.ptr, buffer.length);
} ();
cenforce(size != -1, to!string(link));

if(size <= bufferLen - maxCodeUnits)
return to!string(buffer[0 .. size]);

auto dynamicBuffer = new char[](bufferLen * 3 / 2);

foreach(i; 0 .. 10)
Copy link
Member

@DmitryOlshansky DmitryOlshansky Apr 26, 2016

Choose a reason for hiding this comment

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

Why 10 ? And then what? At least a comment could help a bit

Copy link
Contributor Author

@aG0aep6G aG0aep6G Apr 26, 2016

Choose a reason for hiding this comment

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

Ditto. I just indented this.

{
size = () @trusted {
return posixReadlink(linkz, dynamicBuffer.ptr,
dynamicBuffer.length);
} ();
cenforce(size != -1, to!string(link));

foreach(i; 0 .. 10)
{
size = trustedReadlink(link, dynamicBuffer);
cenforce(size != -1, link);
if(size <= dynamicBuffer.length - maxCodeUnits)
{
dynamicBuffer.length = size;
return () @trusted {
return assumeUnique(dynamicBuffer);
} ();
}

if(size <= dynamicBuffer.length - maxCodeUnits)
{
dynamicBuffer.length = size;
return trustedAssumeUnique(dynamicBuffer);
dynamicBuffer.length = dynamicBuffer.length * 3 / 2;
}

dynamicBuffer.length = dynamicBuffer.length * 3 / 2;
throw new FileException(to!string(link), "Path is too long to read.");
}

throw new FileException(to!string(link), "Path is too long to read.");
}

version(Posix) @safe unittest
Expand All @@ -2434,6 +2467,27 @@ version(Posix) @safe unittest
assertThrown!FileException(readLink("/doesnotexist"));
}

version(Posix) @safe unittest
{
static assert(__traits(compiles, readLink(TestAliasedString("foo"))));
}

version(Posix) unittest // input range of dchars
{
mkdirRecurse(deleteme);
scope(exit) if(deleteme.exists) rmdirRecurse(deleteme);
write(deleteme ~ "/f", "");
import std.range.interfaces: InputRange, inputRangeObject;
import std.utf: byChar;
immutable string link = deleteme ~ "/l";
symlink("f", link);
InputRange!dchar linkr = inputRangeObject(link);
alias R = typeof(linkr);
static assert(isInputRange!R);
static assert(!isForwardRange!R);
assert(readLink(linkr) == "f");
}


/****************************************************
* Get the current working directory.
Expand Down