httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From William A Rowe Jr <wr...@rowe-clan.net>
Subject Re: svn commit: r1729929 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/http_core.h include/httpd.h server/core.c
Date Fri, 12 Feb 2016 23:37:58 GMT
On Feb 12, 2016 04:45, "Yann Ylavic" <ylavic.dev@gmail.com> wrote:
>
> On Fri, Feb 12, 2016 at 10:49 AM, William A Rowe Jr <wrowe@rowe-clan.net>
wrote:
> > On Feb 12, 2016 2:24 AM, "Yann Ylavic" <ylavic.dev@gmail.com> 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.

Well we simply can't reorder bitfields or add them in 2.4 anyways...

> > 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.

Therefore this is problematic for 2.4, and I added a comment to STATUS that
we drop that note if backporting the patch.  The note applies only to trunk.

> 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).

And that is the noble goal heading toward 2.next (3.0?) but it seems there
is no promise that adding one int foo:2 value following a previously
released int bar:2 member would preserve the bit alignment of bar across
all compilers?  That suggests possibly breaking our ABI contract on a
maintenance release.  So adding discrete bit fields seem very iffy, once we
tag a specific release branch.  (I can see safe exceptions if adding a
group of small ints together in a maintenance release, but you would want
to bookend that with a member of known alignment, an int or * or such.)

> 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.

You are only looking at 2's compliment architectures, httpd isn't Intel
specific :)  But we are promised 1..-1, while the fourth value is -2, or
-0, or 2, or NaN.  Not something to rely on, but we don't need a fourth
value here anyways.

> 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.

We've had good success, they get rearranged and optimized only when it is
convenient for us to break ABI.

Unsigned bitfields are sometimes more compact and get us one more value (we
cannot reliably use a -2 value in a 2 bit int as I mentioned above) but
this particular use doesn't seem problematic to optimize later (in
trunk)... Let's leave it as a full int to avoid future ABI alignment issues
for any 2.4 backport?

Mime
View raw message