Skip to content

sha1: don't inline git_hash_global_init for win32#5039

Merged
ethomson merged 1 commit intomasterfrom
ethomson/win32_hash
Apr 7, 2019
Merged

sha1: don't inline git_hash_global_init for win32#5039
ethomson merged 1 commit intomasterfrom
ethomson/win32_hash

Conversation

@ethomson
Copy link
Copy Markdown
Member

@ethomson ethomson commented Apr 4, 2019

Users of the Win32 hash cannot be inlined, as it uses a static struct.
Don't inline it, but continue to declare the function in the header.

/cc @csware

Users of the Win32 hash cannot be inlined, as it uses a static struct.
Don't inline it, but continue to declare the function in the header.
Copy link
Copy Markdown
Member

@pks-t pks-t left a comment

Choose a reason for hiding this comment

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

👍 This is basically the same thing I do in my multi-hash PR.

@pks-t
Copy link
Copy Markdown
Member

pks-t commented Apr 5, 2019

Waiting for @csware to confirm it fixes his problem.

@pks-t
Copy link
Copy Markdown
Member

pks-t commented Apr 5, 2019

By the way, we could also just go all the way and merge #4438, as it also moves all hash implementations into their respective C files.

@csware
Copy link
Copy Markdown
Contributor

csware commented Apr 5, 2019

Works.

@ethomson
Copy link
Copy Markdown
Member Author

ethomson commented Apr 7, 2019

By the way, we could also just go all the way and merge #4438, as it also moves all hash implementations into their respective C files.

Aha, sorry. I thought that #4438 was sort of holding until we were going to add additional hashes. I'm going to go ahead and merge this to unblock @csware but I agree that we should give that some more consideration in the near term.

@ethomson ethomson merged commit c4cd69b into master Apr 7, 2019
@pks-t
Copy link
Copy Markdown
Member

pks-t commented Apr 8, 2019 via email

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.

3 participants