fix mbstring.c -Wsingle-bit-bitfield-constant-conversion #12327
Conversation
|
@morrisonlevi Thanks for spotting this! The code which you are changing here is old code from 2003, which neither I nor anyone else has touched since then. I don't see the point of using a @morrisonlevi What do you think? |
|
@morrisonlevi Would you like to adjust this PR as per above message? If you are busy and would prefer someone else handle this issue, I can do so. |
|
Sure, I can do that. Should have it updated today. |
These were both local variables, so there isn't much value in using bitfields in the first place.
caf731f to
ddc2eab
Compare
|
Looks better, I am just trying to figure out if It seems to indicate whether the headers provided by the user include Content-Type or not. If not, then the function automatically adds a Content-Type header on top of whatever headers the user provided. @morrisonlevi Does that agree with your understanding of these variables? |
|
Yes. Would you prefer that I didn't do any renaming? At least then it's whatever the original author preferred, with less chance of being wrong. |
|
@morrisonlevi I like the fact that you have made the names more readable, but I think the original verb "suppress" more accurately describes the function of these variables. |
|
@morrisonlevi Should I merge this, or will you do so? (Sorry, I don't know if you are a commiter or not.) |
|
@morrisonlevi Thank you very much! |
This opts to fix it in the same way as GH-11729 did, by converting it to a boolean.
I don't see why these would need to be bitfields at all, they could just be booleans as it's an unnamed struct and the struct as a whole isn't used for anything. However, I am a bit tired and it's a Friday afternoon, and we're in RC phase already, so I just did the safer thing.
Edit: as suggested in review, I've updated the PR to just make them local variables.