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

Fixed some missing %p and some pointer arithmetic issues #35

Merged
merged 16 commits into from
Dec 23, 2020

Conversation

j256
Copy link
Owner

@j256 j256 commented Dec 18, 2020

Thanks to @aitap .

@j256
Copy link
Owner Author

j256 commented Dec 23, 2020

Does this pull request fix the issues @aitap ? It works around the necessity of %lld support: #34

@j256 j256 merged commit db05c3a into master Dec 23, 2020
@j256 j256 deleted the gw-fixed-more-pointer-issues branch December 23, 2020 16:16
@aitap
Copy link
Contributor

aitap commented Dec 23, 2020

Sorry for not responding right away and for this issue being so persistent.

The only serious warning left on 64-bit Windows is:

In file included from env.c:44:0:
env.c: In function ‘_dmalloc_address_break’:
env.c:115:23: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
   SET_POINTER(addr_p, (DMALLOC_PNT)hex_to_long(addr_all));

...because on 64-bit Windows, sizeof(long) < sizeof(void*) and a 64-bit hex string denoting a pointer might not fit in a long. A fix I suggested is here, part of a larger commit, but there are probably other ways to solve it.

There are warnings left on 32-bit Windows, but since it's sizeof(PNT_ARITH_TYPE) > sizeof(void*) there, they are probably harmless and can probably be left alone in the interest of portability to the rest of the world with pointer-sized longs.

@j256
Copy link
Owner Author

j256 commented Dec 23, 2020

Thanks @aitap . Can you check out #38? Does that resolve other warnings. I'm working on env.c now.

@aitap
Copy link
Contributor

aitap commented Dec 23, 2020

I should probably warn you that the same hex_to_long function exists in dmalloc_t.c, so it'll need an identical fix.

@j256
Copy link
Owner Author

j256 commented Dec 24, 2020

Thanks @aitap . I got both of them.

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