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.

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

Jump to bottom

Fix nil pointer in etcd tools #6938

Merged
merged 1 commit into from
Apr 17, 2015
Merged

Fix nil pointer in etcd tools #6938

merged 1 commit into from
Apr 17, 2015

Conversation

derekwaynecarr
Copy link
Member

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project, in which case you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

smarterclayton
smarterclayton reviewed Apr 16, 2015
View reviewed changes
pkg/tools/etcd_helper.go
@@ -164,6 +164,10 @@ func (h *EtcdHelper) ExtractObjToList(key string, listObj runtime.Object) error
return err
}

if response == nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you put this inside if err != nil, and have if IsEtcdNotFound(err) return nil? Would be clearer.

@roberthbailey
Copy link
Contributor

I ran across this in #6949 and fixed it differently. I'm happy to take this change instead of mine if this way makes more sense.

@smarterclayton
Copy link
Contributor

The fix i asked Derek for is slightly different and is a bit more consistent with other code. Don't care which pull fixes it though.

On Apr 16, 2015, at 9:42 PM, Robert Bailey notifications@github.com wrote:

I ran across this in #6949 and fixed it differently. I'm happy to take this change instead of mine if this way makes more sense.

—
Reply to this email directly or view it on GitHub.

@roberthbailey
Copy link
Contributor

SGTM. I'll rebase once this PR is merged.

@derekwaynecarr
Copy link
Member Author

@smarterclayton merge it ;-)

@roberthbailey
Copy link
Contributor

+1

smarterclayton added a commit that referenced this pull request Apr 17, 2015
@smarterclayton
Merge pull request #6938 from derekwaynecarr/fix_nil_pointer
153a362 
Fix nil pointer in etcd tools
@smarterclayton smarterclayton merged commit 153a362 into kubernetes:master Apr 17, 2015
@smarterclayton smarterclayton added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 17, 2015
@roberthbailey roberthbailey mentioned this pull request Apr 17, 2015
Closed
@derekwaynecarr derekwaynecarr deleted the fix_nil_pointer branch April 17, 2015 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

kube-apiserver panic information in logs when doing a list with fieldSelector that single matches
4 participants
@derekwaynecarr @googlebot @roberthbailey @smarterclayton