Skip to content

odb: Fix odb foreach to also close on positive error code#4949

Merged
ethomson merged 1 commit intolibgit2:masterfrom
zlikavac32:fix-odb-foreach-cb-positive-error-code
Jan 20, 2019
Merged

odb: Fix odb foreach to also close on positive error code#4949
ethomson merged 1 commit intolibgit2:masterfrom
zlikavac32:fix-odb-foreach-cb-positive-error-code

Conversation

@zlikavac32
Copy link
Copy Markdown
Contributor

Hi,

This should be fix for #4946 .

I'm not familiar with internals, but all tests pass and I did add one new for this case. I don't like that copy/paste, so I was thinking about using struct (or 2 int array) so that only one callback exists in test. What do you think about that?

Also, should anything else be changed?

Thank you.

In include/git2/odb.h it states that callback can also return
positive value which should break looping.

Implementations of git_odb_foreach() and pack_backend__foreach()
did not respect that.
@zlikavac32 zlikavac32 changed the title odb: WI:Fix odb foreach to also close on positive error code odb: WIP: Fix odb foreach to also close on positive error code Jan 20, 2019
@ethomson
Copy link
Copy Markdown
Member

Hiya - thanks for the PR. This looks good to me, I'm fine with a little copypasta in our tests, I don't think there's a big need to use a struct instead.

You put WIP in the title - I'm happy to merge this, but wanted to make sure you were 👍 before I did.

@zlikavac32
Copy link
Copy Markdown
Contributor Author

@ethomson I'm ok, If you're ok :). WIP was more because this is my first PR here, so perhaps I'm missing something out. I'll remove it.

Thanks

@zlikavac32 zlikavac32 changed the title odb: WIP: Fix odb foreach to also close on positive error code odb: Fix odb foreach to also close on positive error code Jan 20, 2019
@ethomson
Copy link
Copy Markdown
Member

🎉 Thanks so much, this is a strict improvement.

@ethomson ethomson merged commit 6b2cd0e into libgit2:master Jan 20, 2019
@ethomson
Copy link
Copy Markdown
Member

Also:

this is my first PR here

Welcome to the libgit2 project. 😀

@zlikavac32
Copy link
Copy Markdown
Contributor Author

@ethomson No problem, thank you for your quick response. Now I can continue with your master :D

Welcome to the libgit2 project.

Thank you, I like this lib. No doubt I'll contribute again when I encounter something :)

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