Willy, Am 18.01.20 um 07:51 schrieb Willy Tarreau: >> - Non-ints are not allowed by the C standard. > > Hmmm that's not exactly what I'm reading in C99 #6.7.2.1 here, which > explicitly permits implementation-defined types: > > "A bit-field shall have a type that is a qualified or unqualified version > of > _Bool, signed int, unsigned int, or some other implementation-defined > type." > > And the implementation we rely on (gcc) says: > > > https://gcc.gnu.org/onlinedocs/gcc/Structures-unions-enumerations-and-bit-fields-implementation.html > Allowable bit-field types other than _Bool, signed int, and unsigned int > (C99 and C11 6.7.2.1). Other integer types, such as long int, and > enumerated types are permitted even in strictly conforming mode.
Fair enough. In the specific implementation of gcc it might be okay. But that doesn't say anything about clang (but I guess it's okay as well). > And this is not surprizing for both tools, because I think that with the > change, the unsigned int field is merged with the previous chars, which > is particularly dirty. And if we place the field before the chars, it > leaves a two-bytes hole between them. > > Ideally I'd use flags there, though we'd have to place them in an unsigned > char. If you're interested in having a look at such a change I'd prefer it, > otherwise I'd prefer not to change what exists for this part because it does > exactly what we need and the change makes it wrong at least debug-wise. I'm not interested. The change to int from char was just a by-product of the signedness changes I made. As you might imagine I was working off the output of some static analyzer which complained that the size `1` bitfield might not be what was meant. While I heard about the existence of bitfields before I have no idea how they worked, read a bit about them and then changed them all to be consistently `int`, because that's what's guaranteed by the standard. Long story short: If you prefer these to remain chars, then so be it. > For this reason I'd like to get the signed-vs-unsigned part merged. Feel free to edit around in my patch to remove the bits that change from something to `int` and leave the adding of the `unsigned` in there. Or drop my patch, create your own one and add me as Co-authored-by or Reported-by or whatever. While doing so please check on the `struct pat_time` I mentioned in my previous email. > Did you check whether this produced any output code change ? It would > be a hint that we might possibly have latent bugs. No, because this is way outside the area of my expertise :-) Best regards Tim Düsterhus
