Skip to content

Address some System.Formats.Tar TODOs (infra and syscalls) #69107

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
merged 22 commits into from
May 17, 2022
Merged

Address some System.Formats.Tar TODOs (infra and syscalls) #69107

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
c178925
Fix Design time build errors by removing src project as dependency of…
carlossanlop May 4, 2022
9f020bd
Add Browser to target platform identifiers. Ensure Browser can consum…
carlossanlop May 4, 2022
c4d444b
Remove nullable enable from csproj since it's now default
carlossanlop May 5, 2022
354e40a
Use FileStream constructor with FileMode.CreateNew to detect and thro…
carlossanlop May 9, 2022
d04ec6b
No error checking on syscalls that do not set errno.
carlossanlop May 9, 2022
9cb26f1
Add RDev field in FileStatus and retrieve it with stat/lstat so devma…
carlossanlop May 10, 2022
d94536a
Simplify some File.Open calls with direct FileStream constructor call…
carlossanlop May 10, 2022
cdbbddf
Implement and consume p/invokes to retrieve uname from uid and gname …
carlossanlop May 10, 2022
e343a3b
size_t variables should not be checked for negative values
carlossanlop May 11, 2022
1af51df
FileStream calls simplified to File.OpenRead
carlossanlop May 11, 2022
d845d56
Remove check for RDev > 0
carlossanlop May 11, 2022
94d2cf0
Use dictionary to preserve repeated unames and gnames mapped to uids …
carlossanlop May 11, 2022
90cc58c
Missing documentation in pal_uid.h new PALEXPORT methods.
carlossanlop May 11, 2022
fe9a927
Adjust syscalls to thread-safe ones. Start with stackalloc, then use …
carlossanlop May 12, 2022
a8f5cf6
Make dicts readonly and non-nullable, use TryGetValue
carlossanlop May 12, 2022
498c084
Merge branch 'main' into TarImprovements
carlossanlop May 13, 2022
7b4e7b5
Reuse 'GetNameFromUid' from pal_networking.c, move to pal_uid.c, use …
carlossanlop May 13, 2022
a9090b4
Remove unnecessary comment
carlossanlop May 13, 2022
7c01996
Put TargetFrameworks back in the first position of the PropertyGroup.
carlossanlop May 13, 2022
8de50ad
Address eerhardt suggestions
carlossanlop May 13, 2022
31ab76b
Update src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.Wri…
carlossanlop May 17, 2022
67fe812
Clarify in pal_uid.h methods comments that new memory is returned
carlossanlop May 17, 2022
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: 1 addition & 1 deletion src/libraries/Common/src/Interop/Unix/System.Native/Interop.DeviceFiles.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,6 @@ internal static int CreateCharacterDevice(string pathName, uint mode, uint major
private static partial int MkNod(string pathName, uint mode, uint major, uint minor);

[LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_GetDeviceIdentifiers", SetLastError = true)]
internal static unsafe partial int GetDeviceIdentifiers(ulong dev, uint* majorNumber, uint* minorNumber);
internal static unsafe partial void GetDeviceIdentifiers(ulong dev, uint* majorNumber, uint* minorNumber);
}
}
73 changes: 73 additions & 0 deletions src/libraries/Common/src/Interop/Unix/System.Native/Interop.GNameUName.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Runtime.InteropServices;
using System.Buffers;
using System.Text;
using System;

internal static partial class Interop
{
internal static partial class Sys
{
/// <summary>
/// Gets the group name associated to the specified group ID.
/// </summary>
/// <param name="gid">The group ID.</param>
/// <returns>On success, return a string with the group name. On failure, returns a null string.</returns>
internal static string? GetGName(uint gid) => GetGNameOrUName(gid, isGName: true);

/// <summary>
/// Gets the user name associated to the specified user ID.
/// </summary>
/// <param name="uid">The user ID.</param>
/// <returns>On success, return a string with the user name. On failure, returns a null string.</returns>
internal static string? GetUName(uint uid) => GetGNameOrUName(uid, isGName: false);

private static string? GetGNameOrUName(uint id, bool isGName)
{
// Common max name length, like /etc/passwd, useradd, groupadd
int outputBufferSize = 32;

while (true)
{
byte[] buffer = ArrayPool<byte>.Shared.Rent(outputBufferSize);
try
{
int resultLength = isGName ?
Interop.Sys.GetGName(id, buffer, buffer.Length) :
Interop.Sys.GetUName(id, buffer, buffer.Length);

if (resultLength < 0)
{
// error
return null;
}
else if (resultLength < buffer.Length)
{
// success
return Encoding.UTF8.GetString(buffer, 0, resultLength);
}
}
finally
{
ArrayPool<byte>.Shared.Return(buffer);
}

// Output buffer was too small, loop around again and try with a larger buffer.
outputBufferSize = buffer.Length * 2;

if (outputBufferSize > 256) // Upper limit allowed for login name in kernel
Copy link
Member

Choose a reason for hiding this comment

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

If we are limiting to 256 anyway, can't we just stackalloc byte[256] and use that? Why go through the pain of a loop and ArrayPool Rent/Return?

Copy link
Member

Choose a reason for hiding this comment

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

Unless it's explicitly documented (pretty much, there's some define -- eg NI_MAXHOST for getnameinfo()) I recommend that we use the stackalloc then resize loop, as in this example:

runtime/src/libraries/Common/src/Interop/Unix/System.Native/Interop.GetCwd.cs

Lines 18 to 45 in 43dd0a7

byte* stackBuf = stackalloc byte[DefaultPathBufferSize];
string? result = GetCwdHelper(stackBuf, DefaultPathBufferSize);
if (result != null)
{
return result;
}
// If that was too small, try increasing large buffer sizes
int bufferSize = DefaultPathBufferSize;
while (true)
{
checked { bufferSize *= 2; }
byte[] buf = ArrayPool<byte>.Shared.Rent(bufferSize);
try
{
fixed (byte* ptr = &buf[0])
{
result = GetCwdHelper(ptr, buf.Length);
if (result != null)
{
return result;
}
}
}
finally
{
ArrayPool<byte>.Shared.Return(buf);
}

I did find at least one case in the pal where it mallocs the buffer and resizes it for you, but it seems our pattern is to do all this in managed code.

Ideally it returns the actual length it needs, or you only loop on ERANGE. I'm not sure what is the pattern.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We use a similar loop in ReadLink:

runtime/src/libraries/Common/src/Interop/Unix/System.Native/Interop.ReadLink.cs

Lines 31 to 68 in c2ec86b

internal static string? ReadLink(ReadOnlySpan<char> path)
{
int outputBufferSize = 1024;
// Use an initial buffer size that prevents disposing and renting
// a second time when calling ConvertAndTerminateString.
using var converter = new ValueUtf8Converter(stackalloc byte[1024]);
while (true)
{
byte[] buffer = ArrayPool<byte>.Shared.Rent(outputBufferSize);
try
{
int resultLength = Interop.Sys.ReadLink(
ref MemoryMarshal.GetReference(converter.ConvertAndTerminateString(path)),
buffer,
buffer.Length);
if (resultLength < 0)
{
// error
return null;
}
else if (resultLength < buffer.Length)
{
// success
return Encoding.UTF8.GetString(buffer, 0, resultLength);
}
}
finally
{
ArrayPool<byte>.Shared.Return(buffer);
}
// Output buffer was too small, loop around again and try with a larger buffer.
outputBufferSize = buffer.Length * 2;
}
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unlike the GetCwd and the ReadLink examples, which do not have an upper bound on the length of the buffer, the uname and gname case do have a max length. So I am now more inclined to just use the 256 byte limit.

Copy link
Member

Choose a reason for hiding this comment

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

Where is that length documented?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://serverfault.com/questions/294121/what-is-the-maximum-username-length-on-current-gnu-linux-systems

The above page says that:

Copy link
Member

Choose a reason for hiding this comment

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

The limit is implementation specific, so there should be a resize loop.
256 is a good value for the stackalloc to start with because that is the Linux value.

Copy link
Member

Choose a reason for hiding this comment

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

Note that the Linux kernel only cares about the numeric ids. libc handles the names.

https://github.com/bminor/glibc/blob/master/sysdeps/unix/sysv/linux/bits/local_lim.h#L89-L90 defines this (implementation specific) limit for Linux:

/* Maximum login name length.  This is arbitrary.  */
#define LOGIN_NAME_MAX		256

Copy link
Contributor Author

@carlossanlop carlossanlop May 12, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

There's another upper limit. It's in the code example in the manual: https://pubs.opengroup.org/onlinepubs/9699919799/functions/getgrgid.html

long int initlen = sysconf(_SC_GETGR_R_SIZE_MAX);
size_t len;
if (initlen == -1)
    /* Default initial length. */
    len = 1024;
else
    len = (size_t) initlen;

I can use 1024 as the absolute upper limit that would stop the loop, I think that's large enough (that's the value I got when printing _SC_GETGR_R_SIZE_MAX in Ubuntu).

Copy link
Member

Choose a reason for hiding this comment

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

I can use 1024 as the absolute upper limit that would stop the loop, I think that's large enough (that's the value I got when printing _SC_GETGR_R_SIZE_MAX in Ubuntu).

I don't think that's necessarily the upper limit, eg., this man page for getgrnam_r says

The call
sysconf(_SC_GETGR_R_SIZE_MAX)
returns either -1, without changing errno, or an initial
suggested size for buf. (If this size is too small, the call
fails with ERANGE, in which case the caller can retry with a
larger buffer.)

In the man page for getgrgid it says it in different words, again referring to getgrnam_r

A call to sysconf(_SC_GETGR_R_SIZE_MAX) returns either -1
without changing errno or an initial value suggested for the size
of this buffer.

Although I see Mono just used it (or 1024) and did not resize without provision for resize, here is a bug report where it was not large enough. https://bugs.ruby-lang.org/issues/9600

{
return null;
}
}
}

[LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_GetGName", SetLastError = true)]
private static partial int GetGName(uint uid, byte[] buffer, int bufferSize);

[LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_GetUName", SetLastError = true)]
private static partial int GetUName(uint uid, byte[] buffer, int bufferSize);
}
}
1 change: 1 addition & 0 deletions src/libraries/Common/src/Interop/Unix/System.Native/Interop.Stat.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ internal struct FileStatus
internal long BirthTime;
internal long BirthTimeNsec;
internal long Dev;
internal long RDev;
internal long Ino;
internal uint UserFlags;
}
Expand Down
1 change: 0 additions & 1 deletion src/libraries/System.Formats.Tar/ref/System.Formats.Tar.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFramework>$(NetCoreAppCurrent)</TargetFramework>
<Nullable>enable</Nullable>
</PropertyGroup>

<ItemGroup>
Expand Down
7 changes: 5 additions & 2 deletions src/libraries/System.Formats.Tar/src/System.Formats.Tar.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<TargetFrameworks>$(NetCoreAppCurrent)-windows;$(NetCoreAppCurrent)-Unix;$(NetCoreAppCurrent)</TargetFrameworks>
<Nullable>enable</Nullable>
<TargetFrameworks>$(NetCoreAppCurrent)-windows;$(NetCoreAppCurrent)-Unix;$(NetCoreAppCurrent)-Browser;$(NetCoreAppCurrent)</TargetFrameworks>
</PropertyGroup>

<!-- DesignTimeBuild requires all the TargetFramework Derived Properties to not be present in the first property group. -->
Expand Down Expand Up @@ -58,10 +57,14 @@
<Compile Include="$(CommonPath)Interop\Unix\Interop.Errors.cs" Link="Common\Interop\Unix\System.Native\Interop.Errors.cs" />
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.DeviceFiles.cs" Link="Common\Interop\Unix\System.Native\Interop.DeviceFiles.cs" />
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.FChMod.cs" Link="Common\Interop\Unix\System.Native\Interop.FChMod.cs" />
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.GNameUName.cs" Link="Common\Interop\Unix\System.Native\Interop.GNameUName.cs" />
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.Link.cs" Link="Common\Interop\Unix\System.Native\Interop.Link.cs" />
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.MkFifo.cs" Link="Common\Interop\Unix\System.Native\Interop.MkFifo.cs" />
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.Permissions.cs" Link="Common\Interop\Unix\Interop.Permissions.cs" />
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.Stat.cs" Link="Common\Interop\Unix\Interop.Stat.cs" />
</ItemGroup>
<!-- Unix and Browser specific files -->
<ItemGroup Condition="'$(TargetPlatformIdentifier)' == 'Unix' or '$(TargetPlatformIdentifier)' == 'Browser'">
<Compile Include="$(CommonPath)System\IO\Archiving.Utils.Unix.cs" Link="Common\System\IO\Archiving.Utils.Unix.cs" />
</ItemGroup>
<ItemGroup>
Expand Down
2 changes: 1 addition & 1 deletion src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarEntry.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,7 @@ private void ExtractAsRegularFile(string destinationFileName)
{
Debug.Assert(!Path.Exists(destinationFileName));

FileStreamOptions fileStreamOptions = new FileStreamOptions()
FileStreamOptions fileStreamOptions = new()
{
Access = FileAccess.Write,
Mode = FileMode.CreateNew,
Expand Down
19 changes: 4 additions & 15 deletions src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarFile.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,8 @@ public static void CreateFromDirectory(string sourceDirectoryName, string destin
throw new DirectoryNotFoundException(string.Format(SR.IO_PathNotFound_Path, sourceDirectoryName));
}

if (Path.Exists(destinationFileName))
{
throw new IOException(string.Format(SR.IO_FileExists_Name, destinationFileName));
}

using FileStream fs = File.Create(destinationFileName, bufferSize: 0x1000, FileOptions.None);
// Throws if the destination file exists
using FileStream fs = new(destinationFileName, FileMode.CreateNew, FileAccess.Write);

CreateFromDirectoryInternal(sourceDirectoryName, fs, includeBaseDirectory, leaveOpen: false);
}
Expand Down Expand Up @@ -170,15 +166,8 @@ public static void ExtractToDirectory(string sourceFileName, string destinationD
throw new DirectoryNotFoundException(string.Format(SR.IO_PathNotFound_Path, destinationDirectoryName));
}

FileStreamOptions fileStreamOptions = new()
{
Access = FileAccess.Read,
BufferSize = 0x1000,
Mode = FileMode.Open,
Share = FileShare.Read
};

using FileStream archive = File.Open(sourceFileName, fileStreamOptions);
// FileMode.Open, FileAccess.Read and FileShare.Read are default FileStreamOptions
using FileStream archive = new(sourceFileName, new FileStreamOptions());

ExtractToDirectoryInternal(archive, destinationDirectoryName, overwriteFiles, leaveOpen: false);
}
Expand Down
24 changes: 8 additions & 16 deletions src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarWriter.Unix.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,13 @@ partial void ReadFileFromDiskAndWriteToArchiveStreamAsEntry(string fullPath, str
_ => throw new FormatException(string.Format(SR.TarInvalidFormat, Format)),
};

if ((entryType is TarEntryType.BlockDevice or TarEntryType.CharacterDevice) && status.Dev > 0)
if ((entryType is TarEntryType.BlockDevice or TarEntryType.CharacterDevice) && status.RDev > 0)
{
uint major;
uint minor;
unsafe
{
Interop.CheckIo(Interop.Sys.GetDeviceIdentifiers((ulong)status.Dev, &major, &minor));
Interop.Sys.GetDeviceIdentifiers((ulong)status.RDev, &major, &minor);
}

entry._header._devMajor = (int)major;
Expand All @@ -60,12 +60,11 @@ partial void ReadFileFromDiskAndWriteToArchiveStreamAsEntry(string fullPath, str

entry._header._mode = (status.Mode & 4095); // First 12 bits

entry.Uid = (int)status.Uid;
entry.Gid = (int)status.Gid;
entry._header._uid = (int)status.Uid;
entry._header._gid = (int)status.Gid;

// TODO: Add these p/invokes https://github.com/dotnet/runtime/issues/68230
entry._header._uName = "";// Interop.Sys.GetUName();
entry._header._gName = "";// Interop.Sys.GetGName();
entry._header._uName = Interop.Sys.GetUName(status.Uid) ?? string.Empty;
entry._header._gName = Interop.Sys.GetGName(status.Gid) ?? string.Empty;
Copy link
Member

Choose a reason for hiding this comment

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

You probably want to cache these in a Dictionary to avoid looking them up each time over?

And, I think the user may want to be able to control the the user id and name that get stored in the tar file.

I think it would be meaningful if the API had option classes that control the creation behavior, and extraction behavior. These classes can be extended later on, rather than adding new overloads to the existing methods.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You probably want to cache these in a Dictionary to avoid looking them up each time over?

Good point. If I already know the uname and gname associated to a uid and gid, there's no need to retrieve them again.

And, I think the user may want to be able to control the the user id and name that get stored in the tar file.

This code path is only reached if the user calls TarWriter.WriteEntry(string fileName, string? entryName). If they wish to choose the uname, gname, uid, gid to other values than the default, they can create the TarEntry instance manually, fill out its fields, and use the other overload TarWriter.WriteEntry(TarEntry entry).

I think it would be meaningful if the API had option classes that control the creation behavior, and extraction behavior. These classes can be extended later on, rather than adding new overloads to the existing methods.

I would welcome that 🙂. We can discuss it in a separate issue.


if (entry.EntryType == TarEntryType.SymbolicLink)
{
Expand All @@ -74,16 +73,9 @@ partial void ReadFileFromDiskAndWriteToArchiveStreamAsEntry(string fullPath, str

if (entry.EntryType is TarEntryType.RegularFile or TarEntryType.V7RegularFile)
{
FileStreamOptions options = new()
{
Mode = FileMode.Open,
Access = FileAccess.Read,
Share = FileShare.Read,
Options = FileOptions.None
};

Debug.Assert(entry._header._dataStream == null);
entry._header._dataStream = File.Open(fullPath, options);
// FileMode.Open, FileAccess.Read and FileShare.Read are default FileStreamOptions
entry._header._dataStream = new FileStream(fullPath, new FileStreamOptions());
}

WriteEntry(entry);
Expand Down
4 changes: 1 addition & 3 deletions src/libraries/System.Formats.Tar/tests/System.Formats.Tar.Tests.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
<Compile Include="$(CommonPath)Interop\Unix\Interop.IOErrors.cs" Link="Common\Interop\Unix\Interop.IOErrors.cs" />
<Compile Include="$(CommonPath)Interop\Unix\Interop.Libraries.cs" Link="Common\Interop\Unix\Interop.Libraries.cs" />
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.DeviceFiles.cs" Link="Common\Interop\Unix\System.Native\Interop.DeviceFiles.cs" />
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.GNameUName.cs" Link="Common\Interop\Unix\System.Native\Interop.GNameUName.cs" />
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.Link.cs" Link="Common\Interop\Unix\System.Native\Interop.Link.cs" />
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.MkFifo.cs" Link="Common\Interop\Unix\System.Native\Interop.MkFifo.cs" />
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.Stat.cs" Link="Common\Interop\Unix\System.Native\Interop.Stat.cs" />
Expand All @@ -66,7 +67,4 @@
<ItemGroup>
<PackageReference Include="System.Formats.Tar.TestData" Version="$(SystemFormatsTarTestDataVersion)" />
</ItemGroup>
<ItemGroup>
<ProjectReference Include="..\src\System.Formats.Tar.csproj" />
</ItemGroup>
</Project>
5 changes: 1 addition & 4 deletions src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectory.File.Tests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,7 @@ public void Extract_Archive_File_OverwriteFalse()

string filePath = Path.Join(destination.Path, "file.txt");

using (StreamWriter writer = File.CreateText(filePath))
{
writer.WriteLine("My existence should cause an exception");
}
File.Create(filePath).Dispose();

Assert.Throws<IOException>(() => TarFile.ExtractToDirectory(sourceArchiveFileName, destination.Path, overwriteFiles: false));
}
Expand Down
18 changes: 4 additions & 14 deletions src/libraries/System.Formats.Tar/tests/TarReader/TarReader.File.Tests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -713,13 +713,8 @@ private void Verify_Archive_BlockDevice(PosixTarEntry blockDevice, IReadOnlyDict
Assert.True(blockDevice.ModificationTime > DateTimeOffset.UnixEpoch);
Assert.Equal(expectedFileName, blockDevice.Name);
Assert.Equal(AssetUid, blockDevice.Uid);

// TODO: Figure out why the numbers don't match https://github.com/dotnet/runtime/issues/68230
// Assert.Equal(AssetBlockDeviceMajor, blockDevice.DeviceMajor);
// Assert.Equal(AssetBlockDeviceMinor, blockDevice.DeviceMinor);
// Remove these two temporary checks when the above is fixed
Assert.True(blockDevice.DeviceMajor > 0);
Assert.True(blockDevice.DeviceMinor > 0);
Assert.Equal(AssetBlockDeviceMajor, blockDevice.DeviceMajor);
Assert.Equal(AssetBlockDeviceMinor, blockDevice.DeviceMinor);
Assert.Equal(AssetGName, blockDevice.GroupName);
Assert.Equal(AssetUName, blockDevice.UserName);

Expand Down Expand Up @@ -749,13 +744,8 @@ private void Verify_Archive_CharacterDevice(PosixTarEntry characterDevice, IRead
Assert.True(characterDevice.ModificationTime > DateTimeOffset.UnixEpoch);
Assert.Equal(expectedFileName, characterDevice.Name);
Assert.Equal(AssetUid, characterDevice.Uid);

// TODO: Figure out why the numbers don't match https://github.com/dotnet/runtime/issues/68230
//Assert.Equal(AssetBlockDeviceMajor, characterDevice.DeviceMajor);
//Assert.Equal(AssetBlockDeviceMinor, characterDevice.DeviceMinor);
// Remove these two temporary checks when the above is fixed
Assert.True(characterDevice.DeviceMajor > 0);
Assert.True(characterDevice.DeviceMinor > 0);
Assert.Equal(AssetCharacterDeviceMajor, characterDevice.DeviceMajor);
Assert.Equal(AssetCharacterDeviceMinor, characterDevice.DeviceMinor);
Assert.Equal(AssetGName, characterDevice.GroupName);
Assert.Equal(AssetUName, characterDevice.UserName);

Expand Down
10 changes: 2 additions & 8 deletions src/libraries/System.Formats.Tar/tests/TarTestsBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,15 +99,9 @@ protected static string GetTarFilePath(CompressionMethod compressionMethod, Test
protected static MemoryStream GetTarMemoryStream(CompressionMethod compressionMethod, TestTarFormat format, string testCaseName)
{
string path = GetTarFilePath(compressionMethod, format, testCaseName);
FileStreamOptions options = new()
{
Access = FileAccess.Read,
Mode = FileMode.Open,
Share = FileShare.Read

};
MemoryStream ms = new();
using (FileStream fs = new FileStream(path, options))
// FileMode.Open, FileAccess.Read and FileShare.Read are default FileStreamOptions
using (FileStream fs = new FileStream(path, new FileStreamOptions()))
{
fs.CopyTo(ms);
}
Expand Down
21 changes: 11 additions & 10 deletions src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntry.File.Tests.Unix.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,10 +91,8 @@ public void Add_BlockDevice(TarFormat format)

VerifyPlatformSpecificMetadata(blockDevicePath, entry);

// TODO: Fix how these values are collected, the numbers don't match even though https://github.com/dotnet/runtime/issues/68230
// they come from stat's dev and from the major/minor syscalls
// Assert.Equal(TestBlockDeviceMajor, entry.DeviceMajor);
// Assert.Equal(TestBlockDeviceMinor, entry.DeviceMinor);
Assert.Equal(TestBlockDeviceMajor, entry.DeviceMajor);
Assert.Equal(TestBlockDeviceMinor, entry.DeviceMinor);

Assert.Null(reader.GetNextEntry());
}
Expand Down Expand Up @@ -138,10 +136,8 @@ public void Add_CharacterDevice(TarFormat format)

VerifyPlatformSpecificMetadata(characterDevicePath, entry);

// TODO: Fix how these values are collected, the numbers don't match even though https://github.com/dotnet/runtime/issues/68230
// they come from stat's dev and from the major/minor syscalls
// Assert.Equal(TestCharacterDeviceMajor, entry.DeviceMajor);
// Assert.Equal(TestCharacterDeviceMinor, entry.DeviceMinor);
Assert.Equal(TestCharacterDeviceMajor, entry.DeviceMajor);
Assert.Equal(TestCharacterDeviceMinor, entry.DeviceMinor);

Assert.Null(reader.GetNextEntry());
}
Expand All @@ -161,8 +157,13 @@ partial void VerifyPlatformSpecificMetadata(string filePath, TarEntry entry)

if (entry is PosixTarEntry posix)
{
Assert.Equal(DefaultGName, posix.GroupName);
Assert.Equal(DefaultUName, posix.UserName);
string gname = Interop.Sys.GetGName(status.Gid);
Assert.NotNull(gname);
string uname = Interop.Sys.GetUName(status.Uid);
Assert.NotNull(uname);

Assert.Equal(gname, posix.GroupName);
Assert.Equal(uname, posix.UserName);

if (entry.EntryType is not TarEntryType.BlockDevice and not TarEntryType.CharacterDevice)
{
Expand Down
2 changes: 2 additions & 0 deletions src/native/libs/System.Native/entrypoints.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,8 @@ static const Entry s_sysNative[] =
DllImportEntry(SystemNative_GetEnv)
DllImportEntry(SystemNative_GetEnviron)
DllImportEntry(SystemNative_FreeEnviron)
DllImportEntry(SystemNative_GetUName)
DllImportEntry(SystemNative_GetGName)
};

EXTERN_C const void* SystemResolveDllImport(const char* name);
Expand Down
9 changes: 2 additions & 7 deletions src/native/libs/System.Native/pal_io.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ c_static_assert(PAL_IN_ISDIR == IN_ISDIR);
static void ConvertFileStatus(const struct stat_* src, FileStatus* dst)
{
dst->Dev = (int64_t)src->st_dev;
dst->RDev = (int64_t)src->st_rdev;
dst->Ino = (int64_t)src->st_ino;
dst->Flags = FILESTATUS_FLAGS_NONE;
dst->Mode = (int32_t)src->st_mode;
Expand Down Expand Up @@ -770,23 +771,17 @@ int32_t SystemNative_SymLink(const char* target, const char* linkPath)
return result;
}

int32_t SystemNative_GetDeviceIdentifiers(uint64_t dev, uint32_t* majorNumber, uint32_t* minorNumber)
void SystemNative_GetDeviceIdentifiers(uint64_t dev, uint32_t* majorNumber, uint32_t* minorNumber)
{
dev_t castedDev = (dev_t)dev;
*majorNumber = (uint32_t)major(castedDev);
*minorNumber = (uint32_t)minor(castedDev);
return ConvertErrorPlatformToPal(errno);
}

int32_t SystemNative_MkNod(const char* pathName, uint32_t mode, uint32_t major, uint32_t minor)
{
dev_t dev = (dev_t)makedev(major, minor);

if (errno > 0)
{
return -1;
}

int32_t result;
while ((result = mknod(pathName, (mode_t)mode, dev)) < 0 && errno == EINTR);
return result;
Expand Down
4 changes: 2 additions & 2 deletions src/native/libs/System.Native/pal_io.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ typedef struct
int64_t BirthTime; // time the file was created
int64_t BirthTimeNsec; // nanosecond part
int64_t Dev; // ID of the device containing the file
int64_t RDev; // ID of the device if it is a special file
int64_t Ino; // inode number of the file
uint32_t UserFlags; // user defined flags
} FileStatus;
Expand Down Expand Up @@ -543,9 +544,8 @@ PALEXPORT int32_t SystemNative_SymLink(const char* target, const char* linkPath)

/**
* Given a device ID, extracts the major and minor and components and returns them.
* Return 0 on success; otherwise, returns -1 and errno is set.
*/
PALEXPORT int32_t SystemNative_GetDeviceIdentifiers(uint64_t dev, uint32_t* majorNumber, uint32_t* minorNumber);
PALEXPORT void SystemNative_GetDeviceIdentifiers(uint64_t dev, uint32_t* majorNumber, uint32_t* minorNumber);

/**
* Creates a special or ordinary file.
Expand Down
Loading