Skip to content

Fix Path.GetPathRoot wrong check in Unix - #35734

Merged
carlossanlop merged 4 commits into
dotnet:masterfrom
carlossanlop:FixGetPathRoot
May 13, 2020
Merged

Fix Path.GetPathRoot wrong check in Unix#35734
carlossanlop merged 4 commits into
dotnet:masterfrom
carlossanlop:FixGetPathRoot

Conversation

@carlossanlop

@carlossanlopcarlossanlop commented May 2, 2020

Copy link
Copy Markdown
Contributor

Fixes#35260

The first element in the condition of the conditional operator was wrong: We should return null if the passed path is effectively empty. I made sure the structure looked just like in the other method overload, to avoid confusion.

I added one unit test to verify the particular case in which I found the bug. In my PR for redundant segments I will be adding many more since GetPathRoot is used by the new API.

@carlossanlopcarlossanlop self-assigned this May 2, 2020
@ghost

ghost commented May 2, 2020

Copy link
Copy Markdown

Tagging subscribers to this area: @jozkee
Notify danmosemsft if you want to be subscribed.

@carlossanlop
carlossanlop requested review from jkotas and jozkeeMay 2, 2020 00:49
jkotas
jkotas reviewed May 2, 2020
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/IO/Path.Unix.cs Outdated
jkotas
jkotas reviewed May 2, 2020
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/IO/Path.Unix.cs Outdated
jkotas
jkotas reviewed May 2, 2020
View reviewed changes
Comment threadsrc/libraries/System.Runtime.Extensions/tests/System/IO/PathTests_Unix.cs Outdated
stephentoub
stephentoub reviewed May 2, 2020
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/IO/Path.Unix.cs Outdated
stephentoub
stephentoub approved these changes May 2, 2020
View reviewed changes

@stephentoubstephentoub left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The product fix looks good. I agree with Jan about fixing the test.

jkotas
jkotas reviewed May 3, 2020
View reviewed changes
Comment threadsrc/libraries/System.Runtime.Extensions/tests/System/IO/PathTests_Unix.cs Outdated
@safernsafern mentioned this pull request May 13, 2020
Open
@jaredparjaredpar mentioned this pull request May 13, 2020
Closed
@carlossanlop
carlossanlop merged commit 61c6581 into dotnet:masterMay 13, 2020
@carlossanlop
carlossanlop deleted the FixGetPathRoot branch May 13, 2020 19:24
@ghostghost locked as resolved and limited conversation to collaborators Dec 9, 2020
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jkotasjkotasjkotas left review comments
@stephentoubstephentoubstephentoub approved these changes
@jozkeejozkeeAwaiting requested review from jozkee

Assignees

@carlossanlopcarlossanlop

Labels

area-System.IO

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Different output in Unix between Path.GetPathRoot overloads for redundant segments

4 participants

@carlossanlop@stephentoub@jkotas@Dotnet-GitSync-Bot