Skip to content
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

check port is valid #2501

Merged
merged 1 commit into from Nov 4, 2013
Merged

check port is valid #2501

merged 1 commit into from Nov 4, 2013

Conversation

vieux
Copy link
Contributor

@vieux vieux commented Nov 1, 2013

Fixes #2480

if _, err := strconv.ParseUint(containerPort, 10, 16); err != nil {
return nil, nil, fmt.Errorf("Invalid containerPort: %s", containerPort)
}
if _, err := strconv.ParseUint(hostPort, 10, 16); hostPort != "" && err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't you check if hostPort is != "" before even trying the strconv?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's the same, and it smaller that way.
Otherwise you should add a new if

@crosbymichael
Copy link
Contributor

LGTM

1 similar comment
@creack
Copy link
Contributor

creack commented Nov 4, 2013

LGTM

creack added a commit that referenced this pull request Nov 4, 2013
@creack creack merged commit 261c2e2 into master Nov 4, 2013
@creack creack deleted the 2480-check_port-fix branch November 4, 2013 19:23
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.

run -p flag accepts variable formats and could become confusing
3 participants