Skip to content

Commit 42778af

Browse files
committed
Address PR feedback
1 parent 55feab8 commit 42778af

File tree

6 files changed

+38
-24
lines changed

6 files changed

+38
-24
lines changed

src/libraries/Common/src/System/IO/Archiving.Utils.Windows.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,8 @@ public static unsafe string EntryFromPath(ReadOnlySpan<char> path, bool appendPa
6262
}
6363

6464
#pragma warning disable CS8500 // takes address of managed type
65-
return string.Create(appendPathSeparator ? path.Length + 1 : path.Length, (appendPathSeparator, RosPtr: (IntPtr)(&path)), static (dest, state) =>
65+
ReadOnlySpan<char> tmpPath = path; // avoid address exposing the span and impacting the other code in the method that uses it
66+
return string.Create(appendPathSeparator ? tmpPath.Length + 1 : tmpPath.Length, (appendPathSeparator, RosPtr: (IntPtr)(&tmpPath)), static (dest, state) =>
6667
{
6768
var path = *(ReadOnlySpan<char>*)state.RosPtr;
6869
path.CopyTo(dest);

src/libraries/System.Private.CoreLib/src/System/IO/Path.cs

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -713,53 +713,66 @@ private static unsafe string JoinInternal(ReadOnlySpan<char> first, ReadOnlySpan
713713
{
714714
Debug.Assert(first.Length > 0 && second.Length > 0 && third.Length > 0 && fourth.Length > 0, "should have dealt with empty paths");
715715

716-
byte firstNeedsSeparator = PathInternal.IsDirectorySeparator(first[^1]) || PathInternal.IsDirectorySeparator(second[0]) ? (byte)0 : (byte)1;
717-
byte secondNeedsSeparator = PathInternal.IsDirectorySeparator(second[^1]) || PathInternal.IsDirectorySeparator(third[0]) ? (byte)0 : (byte)1;
718-
byte thirdNeedsSeparator = PathInternal.IsDirectorySeparator(third[^1]) || PathInternal.IsDirectorySeparator(fourth[0]) ? (byte)0 : (byte)1;
719-
720716
#pragma warning disable CS8500 // This takes the address of, gets the size of, or declares a pointer to a managed type
717+
var state = new JoinInternalState
718+
{
719+
ReadOnlySpanPtr1 = (IntPtr)(&first),
720+
ReadOnlySpanPtr2 = (IntPtr)(&second),
721+
ReadOnlySpanPtr3 = (IntPtr)(&third),
722+
ReadOnlySpanPtr4 = (IntPtr)(&fourth),
723+
NeedSeparator1 = PathInternal.IsDirectorySeparator(first[^1]) || PathInternal.IsDirectorySeparator(second[0]) ? (byte)0 : (byte)1,
724+
NeedSeparator2 = PathInternal.IsDirectorySeparator(second[^1]) || PathInternal.IsDirectorySeparator(third[0]) ? (byte)0 : (byte)1,
725+
NeedSeparator3 = PathInternal.IsDirectorySeparator(third[^1]) || PathInternal.IsDirectorySeparator(fourth[0]) ? (byte)0 : (byte)1,
726+
};
727+
721728
return string.Create(
722-
first.Length + second.Length + third.Length + fourth.Length + firstNeedsSeparator + secondNeedsSeparator + thirdNeedsSeparator,
723-
(FirstRos: (IntPtr)(&first), SecondRos: (IntPtr)(&second), ThirdRos: (IntPtr)(&third), FourthRos: (IntPtr)(&fourth), firstNeedsSeparator, secondNeedsSeparator, thirdNeedsSeparator),
729+
first.Length + second.Length + third.Length + fourth.Length + state.NeedSeparator1 + state.NeedSeparator2 + state.NeedSeparator3,
730+
state,
724731
static (destination, state) =>
725732
{
726-
ReadOnlySpan<char> first = *(ReadOnlySpan<char>*)state.FirstRos;
733+
ReadOnlySpan<char> first = *(ReadOnlySpan<char>*)state.ReadOnlySpanPtr1;
727734
first.CopyTo(destination);
728735
destination = destination.Slice(first.Length);
729736

730-
if (state.firstNeedsSeparator != 0)
737+
if (state.NeedSeparator1 != 0)
731738
{
732739
destination[0] = PathInternal.DirectorySeparatorChar;
733740
destination = destination.Slice(1);
734741
}
735742

736-
ReadOnlySpan<char> second = *(ReadOnlySpan<char>*)state.SecondRos;
743+
ReadOnlySpan<char> second = *(ReadOnlySpan<char>*)state.ReadOnlySpanPtr2;
737744
second.CopyTo(destination);
738745
destination = destination.Slice(second.Length);
739746

740-
if (state.secondNeedsSeparator != 0)
747+
if (state.NeedSeparator2 != 0)
741748
{
742749
destination[0] = PathInternal.DirectorySeparatorChar;
743750
destination = destination.Slice(1);
744751
}
745752

746-
ReadOnlySpan<char> third = *(ReadOnlySpan<char>*)state.ThirdRos;
753+
ReadOnlySpan<char> third = *(ReadOnlySpan<char>*)state.ReadOnlySpanPtr3;
747754
third.CopyTo(destination);
748755
destination = destination.Slice(third.Length);
749756

750-
if (state.thirdNeedsSeparator != 0)
757+
if (state.NeedSeparator3 != 0)
751758
{
752759
destination[0] = PathInternal.DirectorySeparatorChar;
753760
destination = destination.Slice(1);
754761
}
755762

756-
ReadOnlySpan<char> fourth = *(ReadOnlySpan<char>*)state.FourthRos;
763+
ReadOnlySpan<char> fourth = *(ReadOnlySpan<char>*)state.ReadOnlySpanPtr4;
757764
Debug.Assert(fourth.Length == destination.Length);
758765
fourth.CopyTo(destination);
759766
});
760767
#pragma warning restore CS8500
761768
}
762769

770+
private struct JoinInternalState // used to avoid rooting ValueTuple`7
771+
{
772+
public IntPtr ReadOnlySpanPtr1, ReadOnlySpanPtr2, ReadOnlySpanPtr3, ReadOnlySpanPtr4;
773+
public byte NeedSeparator1, NeedSeparator2, NeedSeparator3;
774+
}
775+
763776
private static ReadOnlySpan<byte> Base32Char => "abcdefghijklmnopqrstuvwxyz012345"u8;
764777

765778
internal static unsafe void Populate83FileNameFromRandomBytes(byte* bytes, int byteCount, Span<char> chars)

src/libraries/System.Private.Uri/src/System/Uri.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3841,7 +3841,8 @@ private static unsafe ParsingError CheckSchemeSyntax(ReadOnlySpan<char> span, re
38413841

38423842
// Then look up the syntax in a string-based table.
38433843
#pragma warning disable CS8500 // takes address of managed type
3844-
string str = string.Create(span.Length, (IntPtr)(&span), (buffer, spanPtr) =>
3844+
ReadOnlySpan<char> tmpSpan = span; // avoid address exposing the span and impacting the other code in the method that uses it
3845+
string str = string.Create(tmpSpan.Length, (IntPtr)(&tmpSpan), (buffer, spanPtr) =>
38453846
{
38463847
int charsWritten = (*(ReadOnlySpan<char>*)spanPtr).ToLowerInvariant(buffer);
38473848
Debug.Assert(charsWritten == buffer.Length);

src/libraries/System.Private.Uri/src/System/UriHelper.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -544,11 +544,11 @@ internal static unsafe string StripBidiControlCharacters(ReadOnlySpan<char> strT
544544
}
545545

546546
#pragma warning disable CS8500 // takes address of managed type
547-
return string.Create(strToClean.Length - charsToRemove, (IntPtr)(&strToClean), static (buffer, strToCleanPtr) =>
547+
ReadOnlySpan<char> tmpStrToClean = strToClean; // avoid address exposing the span and impacting the other code in the method that uses it
548+
return string.Create(tmpStrToClean.Length - charsToRemove, (IntPtr)(&tmpStrToClean), static (buffer, strToCleanPtr) =>
548549
{
549-
var strToClean = *(ReadOnlySpan<char>*)strToCleanPtr;
550550
int destIndex = 0;
551-
foreach (char c in strToClean)
551+
foreach (char c in *(ReadOnlySpan<char>*)strToCleanPtr)
552552
{
553553
if (!IsBidiControlCharacter(c))
554554
{

src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/RegexCharClass.cs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1537,22 +1537,20 @@ internal static unsafe string CharsToStringClass(ReadOnlySpan<char> chars)
15371537

15381538
// Get the pointer/length of the span to be able to pass it into string.Create.
15391539
#pragma warning disable CS8500 // takes address of managed type
1540+
ReadOnlySpan<char> tmpChars = chars; // avoid address exposing the span and impacting the other code in the method that uses it
15401541
#if REGEXGENERATOR
15411542
return StringExtensions.Create(
15421543
#else
15431544
return string.Create(
15441545
#endif
1545-
SetStartIndex + count, (IntPtr)(&chars), static (span, charsPtr) =>
1546+
SetStartIndex + count, (IntPtr)(&tmpChars), static (span, charsPtr) =>
15461547
{
1547-
// Reconstruct the span now that we're inside of the lambda.
1548-
ReadOnlySpan<char> chars = *(ReadOnlySpan<char>*)charsPtr;
1549-
15501548
// Fill in the set string
15511549
span[FlagsIndex] = (char)0;
15521550
span[SetLengthIndex] = (char)(span.Length - SetStartIndex);
15531551
span[CategoryLengthIndex] = (char)0;
15541552
int i = SetStartIndex;
1555-
foreach (char c in chars)
1553+
foreach (char c in *(ReadOnlySpan<char>*)charsPtr)
15561554
{
15571555
span[i++] = c;
15581556
if (c != LastChar)

src/libraries/System.Text.RegularExpressions/src/System/Text/SegmentStringBuilder.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,8 @@ public override unsafe string ToString()
7575
}
7676

7777
#pragma warning disable CS8500 // takes address of managed type
78-
string result = string.Create(length, (IntPtr)(&span), static (dest, spanPtr) =>
78+
ReadOnlySpan<ReadOnlyMemory<char>> tmpSpan = span; // avoid address exposing the span and impacting the other code in the method that uses it
79+
string result = string.Create(length, (IntPtr)(&tmpSpan), static (dest, spanPtr) =>
7980
{
8081
Span<ReadOnlyMemory<char>> span = *(Span<ReadOnlyMemory<char>>*)spanPtr;
8182
for (int i = 0; i < span.Length; i++)

0 commit comments

Comments
 (0)