Skip to content

fix regression from #4092#4133

Merged
pks-t merged 1 commit intolibgit2:masterfrom
stinb:khash-cleanup-regression
Feb 24, 2017
Merged

fix regression from #4092#4133
pks-t merged 1 commit intolibgit2:masterfrom
stinb:khash-cleanup-regression

Conversation

@hackhaslam
Copy link
Copy Markdown
Contributor

This is a crash on 32-bit and I assume that it doesn't do the right thing on
64-bit either. MSVC emits a warning for this, but of course, it's easy to get
lost among all of the similar 'possible loss of data' warnings.

@pks-t
Copy link
Copy Markdown
Member

pks-t commented Feb 22, 2017

Agreed. I somehow overlooked the fact that the map_foreach function does not loop over the indices anymore but instead over values. Interesting that tests didn't catch this issue until now.

Could you please use a more descriptive commit message? It becomes somewhat hard to read when you e.g. do a git log, as the user would first need to check #4092 to see what was really changed. Something along the lines of pack: fix looping over cache entries would be much easier to read here :)

Thanks for the fix! Will merge after the amended message

Fixes a regression from libgit2#4092. This is a crash on 32-bit and I assume that
it doesn't do the right thing on 64-bit either. MSVC emits a warning for this,
but of course, it's easy to get lost among all of the similar 'possible loss
of data' warnings.
@hackhaslam hackhaslam force-pushed the khash-cleanup-regression branch from 3023c9f to 685f225 Compare February 22, 2017 16:29
@hackhaslam
Copy link
Copy Markdown
Contributor Author

Okay, I changed the commit message.

@pks-t pks-t merged commit 7f875fb into libgit2:master Feb 24, 2017
@pks-t
Copy link
Copy Markdown
Member

pks-t commented Feb 24, 2017

Thanks a lot! 👍

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