Sitelet https://github.com/microsoft/hcsshim/pull/469
Skip to content

internal/safeopen: fix strings.Contains args order - #469

Merged
Justin (jterry75) merged 1 commit into
microsoft:masterfrom
quasilyte:patch-1
Feb 3, 2019
Merged

Justin (jterry75) merged 1 commit into
microsoft:masterfrom
quasilyte:patch-1

Conversation

@quasilyte

@quasilyte quasilyte (quasilyte) commented Feb 2, 2019 •

Copy link
Copy Markdown
Contributor

The following (old) code:

strings.Contains(":", path)

Only returns true if path is ":" or an empty string.
This is not what was intended.
The proper ":" check is:

strings.Contains(path, ":")

Found by using new go-critic linter check.

Signed-off-by: Iskander Sharipov quasilyte@gmail.com

The following (old) code:

  strings.Contains(":", path)

Only returns true if path is ":" or an empty string.
This is not what was intended.
The proper ":" check is:

  strings.Contains(path, ":")

Found by using new go-critic linter check.

Signed-off-by: Iskander Sharipov <quasilyte@gmail.com>

@lowenna John Howard (lowenna) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice find. Seems the right thing to me. Joe Terry (@jterry) WDYT? I think the intention was as per this change, but haven’t dug or verified.

@jterry75

Copy link
Copy Markdown
Contributor

This was for sure a bug. LGTM

@jterry75
Justin (jterry75) merged commit f92b8fb into microsoft:master Feb 3, 2019
@quasilyte
quasilyte (quasilyte) deleted the patch-1 branch February 4, 2019 07:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants