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

git_array_alloc: return objects of correct type #6008

Merged
merged 1 commit into from Aug 27, 2021
Merged

Conversation

boretrk
Copy link
Contributor

@boretrk boretrk commented Aug 25, 2021

Unlike other array macros git_array_alloc would return a void pointer when an object needed to be allocated.
By changing git_array_grow to only grow the array and make the actual allocation in the git_array_alloc macro the returned type will always be correct.

(((a).size >= (a).asize) ? \
git_array_grow(&(a), sizeof(*(a).ptr)) : \
((a).ptr ? &(a).ptr[(a).size++] : (void *)NULL))
(((a).size < (a).asize || git_array_grow(&(a), sizeof(*(a).ptr)) == 0) ? \
Copy link
Member

Choose a reason for hiding this comment

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

wow this macro is getting unwieldly 😛

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Functionally it would be possible to move the first size-comparison into the the grow-function now, then it will be pretty close to the other macros.
But since inlining relies on compiler extensions for C89 I'd rather keep it where it is since that will prevent git_array_grow from being called most of the time when inlining doesn't work.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, that was more a comment than a criticism. 😁 Thanks for tackling this, these are nice improvements.

@ethomson
Copy link
Member

I think that this makes sense. I'm a little surprised that there's only the one user of git_array_grow.

@boretrk
Copy link
Contributor Author

boretrk commented Aug 27, 2021

I think that this makes sense. I'm a little surprised that there's only the one user of git_array_grow.

Everyone calls git_array_alloc instead.
The allocation has to be made as a macro to be able to extract the item size from the array type.
Then git_array_grow is called as a generic allocator with the item size as an argument, but there is very little reason for any function to call git_array_grow directly.

@ethomson ethomson merged commit 45489a1 into libgit2:main Aug 27, 2021
csware added a commit to csware/libgit2 that referenced this pull request Nov 25, 2021
(fixes issue libgit2#6008)

Signed-off-by: Sven Strickroth <email@cs-ware.de>
csware added a commit to csware/libgit2 that referenced this pull request Nov 25, 2021
(fixes issue libgit2#6008)

Signed-off-by: Sven Strickroth <email@cs-ware.de>
@csware csware mentioned this pull request Nov 25, 2021
csware added a commit to csware/libgit2 that referenced this pull request Nov 25, 2021
(fixes issue libgit2#6008)

Signed-off-by: Sven Strickroth <email@cs-ware.de>
@boretrk boretrk deleted the array branch January 30, 2022 23:42
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.

None yet

2 participants