On Fri, Feb 12, 2016 at 10:49 AM, William A Rowe Jr <[email protected]> wrote: > On Feb 12, 2016 2:24 AM, "Yann Ylavic" <[email protected]> wrote: >> >> I see this TODO about bitfield, but never seen it done in your >> commits, so I wonder where Rainer's warning comes from... >> Anyway, once backported (as you propose), we can hardly change it. > > Precisely, it will NOT change in 2.4 and should not change, and is only > applicable to trunk. I should have omitted from the initial commit. > > If we define this as int :2 and add a second int :2, none of the c standards > promise that the initial 2-bit value maintains its previous alignment. The > alignment is undefined.
Indeed, and as such we can't make any promise about bits alignment/order wrt to the underlying int, and the user can't expect any either. As you noticed, it's not possible to get the address of a bitfield (it makes no sense), and we don't union the bitfields with a true int, so the user would have to do things like "int *bitmask = (int *)&((char *)r)[APR_OFFSETOF(r, useragent_host) + sizeof(r->useragent_host)]" to start playing with the bits, and IMO that deserves a breakage if we add a new bitfield! So I don't thing we should worry about compatibility of bitfields wrt added bits, the goal is to be able to add a new field without increasing the size of the struct (for something that requires only a few bits). Aside note: with an int:2 bitfield, the possible values are 0 (00), 1 (01), -2 (10) and -1 (11), that may be surprising, although that fits your description of double_reverse. IMHO signed bitfields should be avoided, and I don't know if compilers are kind enough (nor allowed) to pack successive signed/unsigned bitfields, probably yes but otherwise we may lose extensibility of "int double_reverse:2" though... So we may prefer "unsigned int double_reverse:2" (and change the description) so that we can later add a single bit that equals 1 and not -1. Regards, Yann.
