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

Minimize casts to pointers #964

Merged
merged 9 commits into from
Oct 25, 2023
Merged

Conversation

cagatay-y
Copy link
Contributor

Minimize casts to pointers by using:

@cagatay-y cagatay-y marked this pull request as ready for review October 24, 2023 17:08
@mkroening mkroening self-assigned this Oct 25, 2023
@mkroening mkroening self-requested a review October 25, 2023 09:41
@mkroening
Copy link
Member

CI failure is being fixed in #965.

Copy link
Member

@mkroening mkroening left a comment

Choose a reason for hiding this comment

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

Thanks a lot! :)

Could you rebase on main instead of merging main? I'd like to avoid merge commits in feature branches.

Also, it would be great if you could have the CI run all of these commits. I usually do that by force pushing the first commit of the feature branch and then pushing commit by commit. Something like git push origin HEAD~10:cast-elimination --force should work (--force can be omitted on all but the first push).

I can also recomment git-interactive-rebase-tool, but any editor of your choice works just as well for rebases.

To print addresses of objects, use address pointer formatter directly
instead of casting to other types to print.
@cagatay-y cagatay-y force-pushed the cast-elimination branch 2 times, most recently from 92d5675 to 3d838c6 Compare October 25, 2023 12:27
@mkroening
Copy link
Member

As a side note, you don't have to wait for CI to finish before pushing the next commit. :)

@mkroening
Copy link
Member

Awesome. You also don't need to be up to date with main and rebase on every change, but having a green CI everywhere would be great. :)

@cagatay-y
Copy link
Contributor Author

As a side note, you don't have to wait for CI to finish before pushing the next commit. :)

I completely misread this message the first time and thought that you said I have to wait :) Thanks for the tip!

@mkroening
Copy link
Member

I did cancel the CI runs starting from the first unsuccessful one, so this does not block the CI. I think, you should be able to do that too if rights are configured correctly: https://github.com/hermit-os/kernel/actions

Using this function makes it clear that the casted object is an exposed
address, rather than a reference or another pointer.
Copy link
Member

@mkroening mkroening left a comment

Choose a reason for hiding this comment

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

Perfect! 👌

Thanks a lot! :)

@mkroening mkroening added this pull request to the merge queue Oct 25, 2023
Merged via the queue into hermit-os:main with commit d42f1b1 Oct 25, 2023
25 checks passed
@cagatay-y cagatay-y deleted the cast-elimination branch October 25, 2023 23:52
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