-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Fix wrong drive format on Unix #107365
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.
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
Fix wrong drive format on Unix #107365
Changes from 1 commit
7e7f368
9fe1985
fec07ce
080825d
1171819
d43dd7b
3542fd8
1607833
2fd5836
49ac57e
eafc91b
4a17349
3505c12
c8c0a84
eba6e78
5da14ac
ab1e2f4
d52fe86
4bbceac
459d1a6
b5e2873
542c244
171c068
1ea7097
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -91,18 +91,19 @@ private static unsafe int GetFormatInfoForMountPoint(string name, out string for | |
| fields.MoveNext(); // Skip Parent ID | ||
| fields.MoveNext(); // Skip Major:Minor | ||
| fields.MoveNext(); // Skip Root | ||
| fields.MoveNext(); // Move next to MountPoint field | ||
|
|
||
| if (!MemoryExtensions.Equals(line[fields.Current], name, StringComparison.Ordinal)) continue; | ||
| if (!line[fields.Current].Equals(name, StringComparison.Ordinal)) continue; | ||
|
|
||
| // Skip to the separator which is end of optional fields (Field 8) | ||
| while (fields.MoveNext() && !MemoryExtensions.Equals(line[fields.Current], "-", StringComparison.Ordinal)); | ||
| while (fields.MoveNext() && !line[fields.Current].Equals("-", StringComparison.Ordinal)) ; | ||
|
|
||
| fields.MoveNext(); | ||
| format = line[fields.Current].ToString(); | ||
|
|
||
| fields.MoveNext(); | ||
| type = GetDriveType(line[fields.Current].ToString()); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't we be passing
|
||
|
|
||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There doesn't seem to be a matching closing |
||
| return 0; | ||
| } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we went over the lines of |
||
| catch { /* ignored */ } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since the format is well documented, we shouldn't need a catch all. We can place this under
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see this was discussed in #107365 (comment). Personally, I think the
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The reason why all/most procfs usage in libs are using try-catch pattern is because anything could go wrong (#62513). Also see #74316 (comment) and #52753 to read on unreliability issues attached with FileSystemInfo.Exists out in the wild.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are some reasons why we don't expect these to occur for Anyway, my main concern here is that a generic catch all masks failures in the parsing. Perhaps we can filter for IOException/UnauthorizedAccessException. These are the exceptions we might expect when not being able to read the procfs file.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. BTW, this approach is similar to what has been used in networking info https://github.com/dotnet/runtime/pull/63696/files#diff-deb3e580b54bccfac2148f60c5d90c6e88b55b508948052e9cf2d1e5421f6ff5R21. Maybe adding a Debug.Fail in catch body for unexpected parsing issues during the development/testing is enough so implementation bugs don't go undetected? |
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.