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

Erros skip nil in Add function #1566

Merged
merged 1 commit into from
Sep 4, 2017
Merged

Conversation

knqyf263
Copy link
Contributor

Make sure these boxes checked before submitting your pull request.

  • Do only one thing
  • No API-breaking changes
  • New code/logic commented & tested
  • Write good commit message, try to squash your commits into a single one
  • Run ./build.sh in gh-pages branch for document changes

For significant changes like big bug fixes, new features, please open an issue to make a agreement on an implementation design/plan first before starting it.

Thank you.

What did this pull request do?

To skip nil in Add .

I wrote the following code.

conn *gorm.DB
...
tx := conn.Begin()
var errs gorm.Errors
errs = errs.Add(tx.Delete(&foo).Error)
errs = errs.Add(tx.Delete(&bar).Error)

if errs.Error() != "" {
  tx.Rollback()
  return
}
...

I got the error.

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x20 pc=0x43e6f75]

I checked errs variable.

gorm.Errors{
  nil,
}

I think Errors doesn't need to have nil.
Or should I always write like this?

if err := tx.Delete(&foo).Error; err != nil {
  errs = errs.Add(err)
}

Is not it supposed that nil will be added?

Thanks!

@jinzhu jinzhu merged commit 6e45625 into go-gorm:master Sep 4, 2017
@jinzhu
Copy link
Member

jinzhu commented Sep 4, 2017

thank you for your PR.

@knqyf263 knqyf263 deleted the handle_nil branch September 6, 2017 05:46
iahmedov pushed a commit to housinganywhere/gorm that referenced this pull request Sep 3, 2018
blefevre pushed a commit to blefevre/gorm that referenced this pull request Feb 17, 2020
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.

2 participants