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

Fix char*/unsigned char*/int8_t*/uint8_t* casting #150

Closed
briansmith opened this issue Mar 12, 2016 · 5 comments
Closed

Fix char*/unsigned char*/int8_t*/uint8_t* casting #150

briansmith opened this issue Mar 12, 2016 · 5 comments

Comments

@briansmith
Copy link
Owner

@regehr brought to attention something that @ch3root pointed out: int8_t isn't necessarily char and uint8_t isn't necessarily unsigned char. The exceptional case in the C11 effective type rules is only for “character types.” Thus, we should never be casting to uint8_t or int8_t.

We do such casts currently, for example, in rsaz_exp.c and a few places in gcm.c. And probably other places.

I think this should be done in three steps:

  1. Finish the work on removing unnecessary casts—hopefully all casts, or at least all pointer casts—in ring.
  2. If there are any remaining casts where this issue is relevant, define a new type aliasing_uint8 as unsigned char and statically assert that it is an 8-bit type. I guess this means statically asserting that CHAR_MAX is 255 and maybe some other stuff.
  3. Document this issue in the style/review guide.

The reference for this is this ancient email to the GCC mailing list: https://gcc.gnu.org/ml/gcc/2000-05/msg01106.html

@regehr
Copy link

regehr commented Mar 12, 2016

@briansmith
Copy link
Owner Author

Thanks John.

I took a look at what it would take to eliminate all the pointer casts in the C code we currently have. It doesn't look too bad! The list is in #151. (Note that we've already done a bunch of work to eliminate casts in ring and so the list in #151 isn't anything like what it would be in other forks of this codebase.)

@regehr
Copy link

regehr commented Mar 12, 2016

Based on GCC 66110 and some tests that I've been running, I'm not sure this is an immediate danger. But wow, what a sucky idea that these nice things from stdint might be really dangerous.

@briansmith
Copy link
Owner Author

briansmith commented Dec 12, 2016

Based on the latest grep through the source as part of writing #151 (comment), it looks like we will indeed be able to remove every single pointer cast in ring's C code. So, there's probably no action required here. Will keep this open until we close out #151.

@briansmith
Copy link
Owner Author

This was done. There are no such pointer casts any more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants