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

Changed unsigned ints and chars to cstdint counterparts #845

Merged
merged 2 commits into from Aug 23, 2014

Conversation

crozhon
Copy link
Contributor

@crozhon crozhon commented Aug 20, 2014

No description provided.

@lioncash
Copy link
Member

@dolphin-emu-bot rebuild

@delroth
Copy link
Member

delroth commented Aug 20, 2014

... that's not cstdint?

@lioncash
Copy link
Member

I think what he meant is that our CommonTypes typedefs utilize the cstdint types as a base, so it's sort of equivalent (albeit ambiguous)

@crozhon
Copy link
Contributor Author

crozhon commented Aug 20, 2014

That is what I meant.

@crozhon
Copy link
Contributor Author

crozhon commented Aug 20, 2014

The typedefs seem completely unnecessary since they are only used in one other place. It seems better to just use our CommonTypes typedefs.

@shuffle2
Copy link
Contributor

This naming is used because the structs are copied from the real ELF support library.

@lioncash
Copy link
Member

@shuffle2 so should the type names be kept? As far as I see, it just adds more noise to the code since it adds more typedefs elsewhere for the same thing.

@shuffle2
Copy link
Contributor

I guess it doesn't matter one way or the other, just mentioning the reasoning.

@lioncash
Copy link
Member

Oh ok.

@Parlane
Copy link
Member

Parlane commented Aug 20, 2014

@dolphin-emu-bot rebuild

@BhaaLseN
Copy link
Member

Aww, now the Elf(32)_Sword is no more! ;(

@delroth
Copy link
Member

delroth commented Aug 23, 2014

@ChuckRozhon: This comment grants you the permission to merge this pull request whenever you think it is ready. After addressing the remaining comments, click this link to merge.


Not really a big fan since this is basically external code - but I can understand the reasoning and I'm not completely opposed to it.

@dolphin-emu-bot allowmerge

dolphin-emu-bot added a commit that referenced this pull request Aug 23, 2014
Changed unsigned ints and chars to cstdint counterparts
@dolphin-emu-bot dolphin-emu-bot merged commit 43f8903 into dolphin-emu:master Aug 23, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
7 participants