build: rename bool->obool

PATCH In review Alon Bar-Lev 4 participants 16 messages Mar 24, 2012 – Apr 28, 2012 1 message not in the archive

lore

Share & tools

lore

A message that is not in the archive (sent off-list or not yet received): [email protected]...

Alon Bar-Lev

author

2012-03-24 19:21 UTC #1 lore raw

Quoted 117 lines · Alon Bar-Lev

PPC architectures and conflict with stdbool.h.
basic.h defines a type 'bool' that conflicts with the altivec keyword bool which has to be fixed upstream, see bugs[1][2].
[1] https://bugs.gentoo.org/show_bug.cgi?id=293840
[2] https://bugs.gentoo.org/show_bug.cgi?id=297854
Signed-off-by Alon Bar-Lev <[email protected]...>
---
src/openvpn/basic.h | 6 +-
 src/openvpn/buffer.c             |   38 ++++----
 src/openvpn/buffer.h             |   80 ++++++++--------
 src/openvpn/clinat.c             |    4 +-
 src/openvpn/console.c            |   22 ++--
 src/openvpn/console.h            |    4 +-
 src/openvpn/crypto.c             |   34 ++++----
 src/openvpn/crypto.h             |   30 +++---
 src/openvpn/crypto_backend.h     |    2 +-
 src/openvpn/crypto_openssl.c     |    6 +-
 src/openvpn/crypto_polarssl.c    |    4 +-
 src/openvpn/error.c              |   24 +++---
 src/openvpn/error.h              |   16 ++--
 src/openvpn/event.c              |   16 ++--
 src/openvpn/fdmisc.c             |    4 +-
 src/openvpn/fdmisc.h             |    4 +-
 src/openvpn/forward.c            |   10 +-
 src/openvpn/forward.h            |    4 +-
 src/openvpn/fragment.c           |    2 +-
 src/openvpn/fragment.h           |    8 +-
 src/openvpn/gremlin.c            |    8 +-
 src/openvpn/gremlin.h            |    2 +-
 src/openvpn/helper.c             |    2 +-
src/openvpn/init.c | 70 +++++++------- src/openvpn/init.h | 16 ++-- src/openvpn/interval.c | 4 +- src/openvpn/interval.h | 12 +- src/openvpn/list.c | 14 ++-- src/openvpn/list.h | 16 ++-- src/openvpn/lzo.c | 8 +- src/openvpn/lzo.h | 6 +- src/openvpn/manage.c | 86 +++++++++--------- src/openvpn/manage.h | 56 ++++++------
 src/openvpn/mbuf.c               |    4 +-
 src/openvpn/mbuf.h               |    6 +-
 src/openvpn/misc.c               |   54 ++++++------
 src/openvpn/misc.h               |   30 +++---
src/openvpn/mroute.c | 24 +++---
src/openvpn/mroute.h | 10 +-
src/openvpn/mtcp.c | 18 ++--
src/openvpn/mtcp.h | 2 +-
src/openvpn/mtu.c | 6 +-
src/openvpn/mtu.h | 8 +-
 src/openvpn/multi.c              |   60 ++++++------
 src/openvpn/multi.h              |   54 ++++++------
 src/openvpn/ntlm.c               |    2 +-
 src/openvpn/occ.c                |    2 +-
 src/openvpn/occ.h                |    2 +-
 src/openvpn/openvpn.c            |    2 +-
 src/openvpn/openvpn.h            |   48 +++++-----
 src/openvpn/options.c            |   84 +++++++++---------
 src/openvpn/options.h            |  190 +++++++++++++++++++-------------------
 src/openvpn/otime.c              |    6 +-
 src/openvpn/otime.h              |   18 ++--
 src/openvpn/packet_id.c          |   22 ++--
 src/openvpn/packet_id.h          |   22 ++--
 src/openvpn/pf-inline.h          |   10 +-
src/openvpn/pf.c | 32 +++---
src/openvpn/pf.h | 14 ++--
src/openvpn/ping.h | 2 +-
src/openvpn/pkcs11.c | 22 ++--
src/openvpn/pkcs11.h | 16 ++--
src/openvpn/platform.c | 16 ++--
src/openvpn/platform.h | 12 +-
src/openvpn/plugin.c | 16 ++--
src/openvpn/plugin.h | 12 +-
 src/openvpn/pool.c               |   22 ++--
 src/openvpn/pool.h               |   18 ++--
 src/openvpn/proto.c              |    2 +-
 src/openvpn/proto.h              |    2 +-
 src/openvpn/proxy.c              |   26 +++---
 src/openvpn/proxy.h              |    8 +-
src/openvpn/ps.c | 12 +-
src/openvpn/ps.h | 2 +-
src/openvpn/push.c | 26 +++---
src/openvpn/push.h | 8 +-
 src/openvpn/pushlist.h           |    2 +-
 src/openvpn/reliable.c           |   36 ++++----
 src/openvpn/reliable.h           |   32 +++---
src/openvpn/route.c | 68 +++++++------- src/openvpn/route.h | 32 +++--- src/openvpn/session_id.h | 10 +- src/openvpn/shaper.c | 4 +- src/openvpn/shaper.h | 4 +- src/openvpn/sig.c | 10 +- src/openvpn/sig.h | 6 +- src/openvpn/socket.c | 94 ++++++++++---------- src/openvpn/socket.h | 98 ++++++++++---------- src/openvpn/socks.c | 8 +- src/openvpn/socks.h | 6 +- src/openvpn/ssl.c | 88 +++++++++--------- src/openvpn/ssl.h | 16 ++--
 src/openvpn/ssl_backend.h        |    8 +-
 src/openvpn/ssl_common.h         |   26 +++---
 src/openvpn/ssl_openssl.c        |   12 +-
src/openvpn/ssl_polarssl.c | 8 +- src/openvpn/ssl_polarssl.h | 2 +- src/openvpn/ssl_verify.c | 26 +++--- src/openvpn/ssl_verify.h | 10 +- src/openvpn/ssl_verify_openssl.c | 4 +- src/openvpn/status.c | 12 +- src/openvpn/status.h | 10 +- src/openvpn/tun.c | 104 ++++++++++---------- src/openvpn/tun.h | 52 +++++----- src/openvpn/win32.c | 28 +++--- src/openvpn/win32.h | 26 +++--- src/plugins/defer/simple.c | 9 +--

Quoted 1 line

 107 files changed, 1208 insertions(+), 1213 deletions(-)

Well,

As usual,
Mailing list limitation for large/complex patches inhibit proper communication.

Patch can be found/pulled from github[1].

[1] https://github.com/alonbl/openvpn/commit/bec27e73f534e97c3d24d63680a0dfe6824aef2a

Fabian Knittel

2012-03-25 10:51 UTC #2 lore raw

Hi Alon,

2012/3/24 Alon Bar-Lev <[email protected]...>:

Quoted 8 lines · Alon Bar-Lev

Quoted 7 lines
PPC architectures and conflict with stdbool.h.
basic.h defines a type 'bool' that conflicts with the altivec keyword bool which has to be fixed upstream, see bugs[1][2].
[1] https://bugs.gentoo.org/show_bug.cgi?id=293840
[2] https://bugs.gentoo.org/show_bug.cgi?id=297854

The approach attempted in [2], using stdbool.h, looks far less intrusive than renaming "bool" to "obool" everywhere, so why did you choose the latter approach? Jan Scholz's comment #6 [3] sounds like the right approach to me ...

Cheers
Fabian

3: https://bugs.gentoo.org/show_bug.cgi?id=297854#c6

Alon Bar-Lev

author

2012-03-25 11:10 UTC #3 lore raw

Quoted 19 lines · Fabian Knittel

Hi Alon,
2012/3/24 Alon Bar-Lev <[email protected]...>:
Quoted 9 lines
Quoted 7 lines · Alon Bar-Lev
PPC architectures and conflict with stdbool.h.
basic.h defines a type 'bool' that conflicts with the altivec keyword bool which has to be fixed upstream, see bugs[1][2].
[1] https://bugs.gentoo.org/show_bug.cgi?id=293840
[2] https://bugs.gentoo.org/show_bug.cgi?id=297854
The approach attempted in [2], using stdbool.h, looks far less intrusive than renaming "bool" to "obool" everywhere, so why did you choose the latter approach? Jan Scholz's comment #6 [3] sounds like the right approach to me ...

Quoted 4 lines

Cheers
Fabian
3: https://bugs.gentoo.org/show_bug.cgi?id=297854#c6

Hello,

This is a good question.

My formal answer is that although stdbool contains the "standard" implication it is far from being standard right now.

First, there is a problem with constant definition of sizeof(bool), and second there is a problem with integration of C and C++ code one using C++ and the other using stdbool constants.

Most platforms, such as Microsoft platform and Sun still missing that functionality anyway.

What I recommend in order to avoid conflicts and resolve this easily is simply not to use the "bool" keywords. This is what I am doing in all of my C projects.

If we want to invest some more time, we can make sure that openvpn does not depend of sizeof(bool) and create emulation for bool for platforms that do not provide it.

However the flags modifications from [1] should be merged anyway, I will submit a patch.

What do you think?

Thanks,
Alon.

[1] https://297854.bugs.gentoo.org/attachment.cgi?id=214304

Gert Doering

2012-03-25 11:15 UTC #4 lore raw

Hi,

Quoted 9 lines · Alon Bar-Lev

However the flags modifications from [1] should be merged anyway, I will submit a patch.
What do you think?
Thanks,
Alon.
[1] https://297854.bugs.gentoo.org/attachment.cgi?id=214304

I strongly support that. This really is used as a bitfield, and not as a boolean, and should not be labled "bool" (and this is independent from the bool/obool/stdbool discussion).

gert

Signature · 4 lines

USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...

Gert Doering

2012-03-25 11:12 UTC #5 lore raw

In reply to Fabian Knittel

Hi,

Quoted 7 lines · Fabian Knittel

2012/3/24 Alon Bar-Lev <[email protected]...>:
Quoted 6 lines · Alon Bar-Lev
Quoted 5 lines
PPC architectures and conflict with stdbool.h.
basic.h defines a type 'bool' that conflicts with the altivec keyword bool which has to be fixed upstream, see bugs[1][2].

Quoted 7 lines

Quoted 2 lines
Quoted 2 lines
[1] https://bugs.gentoo.org/show_bug.cgi?id=293840
[2] https://bugs.gentoo.org/show_bug.cgi?id=297854
The approach attempted in [2], using stdbool.h, looks far less intrusive than renaming "bool" to "obool" everywhere, so why did you choose the latter approach? Jan Scholz's comment #6 [3] sounds like the right approach to me ...

The problem is that stdbool.h seems to define a data type that is not necessarily what the rest of OpenVPN expects (structure sizes etc), which is also mentioned in these two bug reports - read: it will *break* OpenVPN, or at least the plugin interface. So moving to stdbool.h would require careful analysis of every single usage of "bool" to see whether this is really only doing "boolean" stuff, or whether there's other issues lurking here (causing the breakage David noticed in [2]).

Personally I think that this is a regression in GCC, and we should NOT do anything about it. If the compiler is documented to have a new data type "bool__", it should not mess with "bool", which is a different name by all means - bad enough that we have to jump through hoops to care for MSVC, but that's at least a mainstream platform, as opposed to "PPC with a specific compiler option (-maltivec)".

But I'm not insisting on this either way. This is personal optionion, and stuff like that is close to coding style and religion anyway.

gert

Signature · 4 lines

USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...

Alon Bar-Lev

author

2012-03-25 11:16 UTC #6 lore raw

Quoted 6 lines · Gert Doering

Personally I think that this is a regression in GCC, and we should NOT do anything about it. If the compiler is documented to have a new data type "bool__", it should not mess with "bool", which is a different name by all means - bad enough that we have to jump through hoops to care for MSVC, but that's at least a mainstream platform, as opposed to "PPC with a specific compiler option (-maltivec)".

Right.
But I don't think we should be in fight with gcc... These guys are doing a great work, without them no open source could have been manifested.
If they chose to implement this, and this entered the standard, we should either comply or use alternate method.

And it is not difficult to use alternate method, just long patch :)

Alon.

Alon Bar-Lev

author

2012-03-25 11:28 UTC #7 lore raw

In reply to Alon Bar-Lev

Quoted 120 lines · Alon Bar-Lev

Quoted 117 lines · Alon Bar-Lev
PPC architectures and conflict with stdbool.h.
basic.h defines a type 'bool' that conflicts with the altivec keyword bool which has to be fixed upstream, see bugs[1][2].
[1] https://bugs.gentoo.org/show_bug.cgi?id=293840
[2] https://bugs.gentoo.org/show_bug.cgi?id=297854
Signed-off-by Alon Bar-Lev <[email protected]...>
---
src/openvpn/basic.h | 6 +-
 src/openvpn/buffer.c             |   38 ++++----
 src/openvpn/buffer.h             |   80 ++++++++--------
 src/openvpn/clinat.c             |    4 +-
 src/openvpn/console.c            |   22 ++--
 src/openvpn/console.h            |    4 +-
 src/openvpn/crypto.c             |   34 ++++----
 src/openvpn/crypto.h             |   30 +++---
 src/openvpn/crypto_backend.h     |    2 +-
 src/openvpn/crypto_openssl.c     |    6 +-
 src/openvpn/crypto_polarssl.c    |    4 +-
 src/openvpn/error.c              |   24 +++---
 src/openvpn/error.h              |   16 ++--
 src/openvpn/event.c              |   16 ++--
 src/openvpn/fdmisc.c             |    4 +-
 src/openvpn/fdmisc.h             |    4 +-
 src/openvpn/forward.c            |   10 +-
 src/openvpn/forward.h            |    4 +-
 src/openvpn/fragment.c           |    2 +-
 src/openvpn/fragment.h           |    8 +-
 src/openvpn/gremlin.c            |    8 +-
 src/openvpn/gremlin.h            |    2 +-
 src/openvpn/helper.c             |    2 +-
src/openvpn/init.c | 70 +++++++------- src/openvpn/init.h | 16 ++-- src/openvpn/interval.c | 4 +- src/openvpn/interval.h | 12 +- src/openvpn/list.c | 14 ++-- src/openvpn/list.h | 16 ++-- src/openvpn/lzo.c | 8 +- src/openvpn/lzo.h | 6 +- src/openvpn/manage.c | 86 +++++++++--------- src/openvpn/manage.h | 56 ++++++------
 src/openvpn/mbuf.c               |    4 +-
 src/openvpn/mbuf.h               |    6 +-
 src/openvpn/misc.c               |   54 ++++++------
 src/openvpn/misc.h               |   30 +++---
src/openvpn/mroute.c | 24 +++---
src/openvpn/mroute.h | 10 +-
src/openvpn/mtcp.c | 18 ++--
src/openvpn/mtcp.h | 2 +-
src/openvpn/mtu.c | 6 +-
src/openvpn/mtu.h | 8 +-
 src/openvpn/multi.c              |   60 ++++++------
 src/openvpn/multi.h              |   54 ++++++------
 src/openvpn/ntlm.c               |    2 +-
 src/openvpn/occ.c                |    2 +-
 src/openvpn/occ.h                |    2 +-
 src/openvpn/openvpn.c            |    2 +-
 src/openvpn/openvpn.h            |   48 +++++-----
 src/openvpn/options.c            |   84 +++++++++---------
 src/openvpn/options.h            |  190 +++++++++++++++++++-------------------
 src/openvpn/otime.c              |    6 +-
 src/openvpn/otime.h              |   18 ++--
 src/openvpn/packet_id.c          |   22 ++--
 src/openvpn/packet_id.h          |   22 ++--
 src/openvpn/pf-inline.h          |   10 +-
src/openvpn/pf.c | 32 +++---
src/openvpn/pf.h | 14 ++--
src/openvpn/ping.h | 2 +-
src/openvpn/pkcs11.c | 22 ++--
src/openvpn/pkcs11.h | 16 ++--
src/openvpn/platform.c | 16 ++--
src/openvpn/platform.h | 12 +-
src/openvpn/plugin.c | 16 ++--
src/openvpn/plugin.h | 12 +-
 src/openvpn/pool.c               |   22 ++--
 src/openvpn/pool.h               |   18 ++--
 src/openvpn/proto.c              |    2 +-
 src/openvpn/proto.h              |    2 +-
 src/openvpn/proxy.c              |   26 +++---
 src/openvpn/proxy.h              |    8 +-
src/openvpn/ps.c | 12 +-
src/openvpn/ps.h | 2 +-
src/openvpn/push.c | 26 +++---
src/openvpn/push.h | 8 +-
 src/openvpn/pushlist.h           |    2 +-
 src/openvpn/reliable.c           |   36 ++++----
 src/openvpn/reliable.h           |   32 +++---
src/openvpn/route.c | 68 +++++++------- src/openvpn/route.h | 32 +++--- src/openvpn/session_id.h | 10 +- src/openvpn/shaper.c | 4 +- src/openvpn/shaper.h | 4 +- src/openvpn/sig.c | 10 +- src/openvpn/sig.h | 6 +- src/openvpn/socket.c | 94 ++++++++++---------- src/openvpn/socket.h | 98 ++++++++++---------- src/openvpn/socks.c | 8 +- src/openvpn/socks.h | 6 +- src/openvpn/ssl.c | 88 +++++++++--------- src/openvpn/ssl.h | 16 ++--
 src/openvpn/ssl_backend.h        |    8 +-
 src/openvpn/ssl_common.h         |   26 +++---
 src/openvpn/ssl_openssl.c        |   12 +-
src/openvpn/ssl_polarssl.c | 8 +- src/openvpn/ssl_polarssl.h | 2 +- src/openvpn/ssl_verify.c | 26 +++--- src/openvpn/ssl_verify.h | 10 +- src/openvpn/ssl_verify_openssl.c | 4 +- src/openvpn/status.c | 12 +- src/openvpn/status.h | 10 +- src/openvpn/tun.c | 104 ++++++++++---------- src/openvpn/tun.h | 52 +++++----- src/openvpn/win32.c | 28 +++--- src/openvpn/win32.h | 26 +++--- src/plugins/defer/simple.c | 9 +--
Quoted 1 line
 107 files changed, 1208 insertions(+), 1213 deletions(-)

Quoted 8 lines

Well,
As usual,
Mailing list limitation for large/complex patches inhibit proper communication.
Patch can be found/pulled from github[1].
[1] https://github.com/alonbl/openvpn/commit/bec27e73f534e97c3d24d63680a0dfe6824aef2a

NOTICE: I rebased this patch to "[PATCH] cleanup: flags should not be bool" as a base.

Alon.

Alon Bar-Lev

author

2012-03-28 09:32 UTC #8 lore raw

In reply to Alon Bar-Lev

Quoted 120 lines · Alon Bar-Lev

Quoted 117 lines · Alon Bar-Lev
PPC architectures and conflict with stdbool.h.
basic.h defines a type 'bool' that conflicts with the altivec keyword bool which has to be fixed upstream, see bugs[1][2].
[1] https://bugs.gentoo.org/show_bug.cgi?id=293840
[2] https://bugs.gentoo.org/show_bug.cgi?id=297854
Signed-off-by Alon Bar-Lev <[email protected]...>
---
src/openvpn/basic.h | 6 +-
 src/openvpn/buffer.c             |   38 ++++----
 src/openvpn/buffer.h             |   80 ++++++++--------
 src/openvpn/clinat.c             |    4 +-
 src/openvpn/console.c            |   22 ++--
 src/openvpn/console.h            |    4 +-
 src/openvpn/crypto.c             |   34 ++++----
 src/openvpn/crypto.h             |   30 +++---
 src/openvpn/crypto_backend.h     |    2 +-
 src/openvpn/crypto_openssl.c     |    6 +-
 src/openvpn/crypto_polarssl.c    |    4 +-
 src/openvpn/error.c              |   24 +++---
 src/openvpn/error.h              |   16 ++--
 src/openvpn/event.c              |   16 ++--
 src/openvpn/fdmisc.c             |    4 +-
 src/openvpn/fdmisc.h             |    4 +-
 src/openvpn/forward.c            |   10 +-
 src/openvpn/forward.h            |    4 +-
 src/openvpn/fragment.c           |    2 +-
 src/openvpn/fragment.h           |    8 +-
 src/openvpn/gremlin.c            |    8 +-
 src/openvpn/gremlin.h            |    2 +-
 src/openvpn/helper.c             |    2 +-
src/openvpn/init.c | 70 +++++++------- src/openvpn/init.h | 16 ++-- src/openvpn/interval.c | 4 +- src/openvpn/interval.h | 12 +- src/openvpn/list.c | 14 ++-- src/openvpn/list.h | 16 ++-- src/openvpn/lzo.c | 8 +- src/openvpn/lzo.h | 6 +- src/openvpn/manage.c | 86 +++++++++--------- src/openvpn/manage.h | 56 ++++++------
 src/openvpn/mbuf.c               |    4 +-
 src/openvpn/mbuf.h               |    6 +-
 src/openvpn/misc.c               |   54 ++++++------
 src/openvpn/misc.h               |   30 +++---
src/openvpn/mroute.c | 24 +++---
src/openvpn/mroute.h | 10 +-
src/openvpn/mtcp.c | 18 ++--
src/openvpn/mtcp.h | 2 +-
src/openvpn/mtu.c | 6 +-
src/openvpn/mtu.h | 8 +-
 src/openvpn/multi.c              |   60 ++++++------
 src/openvpn/multi.h              |   54 ++++++------
 src/openvpn/ntlm.c               |    2 +-
 src/openvpn/occ.c                |    2 +-
 src/openvpn/occ.h                |    2 +-
 src/openvpn/openvpn.c            |    2 +-
 src/openvpn/openvpn.h            |   48 +++++-----
 src/openvpn/options.c            |   84 +++++++++---------
 src/openvpn/options.h            |  190 +++++++++++++++++++-------------------
 src/openvpn/otime.c              |    6 +-
 src/openvpn/otime.h              |   18 ++--
 src/openvpn/packet_id.c          |   22 ++--
 src/openvpn/packet_id.h          |   22 ++--
 src/openvpn/pf-inline.h          |   10 +-
src/openvpn/pf.c | 32 +++---
src/openvpn/pf.h | 14 ++--
src/openvpn/ping.h | 2 +-
src/openvpn/pkcs11.c | 22 ++--
src/openvpn/pkcs11.h | 16 ++--
src/openvpn/platform.c | 16 ++--
src/openvpn/platform.h | 12 +-
src/openvpn/plugin.c | 16 ++--
src/openvpn/plugin.h | 12 +-
 src/openvpn/pool.c               |   22 ++--
 src/openvpn/pool.h               |   18 ++--
 src/openvpn/proto.c              |    2 +-
 src/openvpn/proto.h              |    2 +-
 src/openvpn/proxy.c              |   26 +++---
 src/openvpn/proxy.h              |    8 +-
src/openvpn/ps.c | 12 +-
src/openvpn/ps.h | 2 +-
src/openvpn/push.c | 26 +++---
src/openvpn/push.h | 8 +-
 src/openvpn/pushlist.h           |    2 +-
 src/openvpn/reliable.c           |   36 ++++----
 src/openvpn/reliable.h           |   32 +++---
src/openvpn/route.c | 68 +++++++------- src/openvpn/route.h | 32 +++--- src/openvpn/session_id.h | 10 +- src/openvpn/shaper.c | 4 +- src/openvpn/shaper.h | 4 +- src/openvpn/sig.c | 10 +- src/openvpn/sig.h | 6 +- src/openvpn/socket.c | 94 ++++++++++---------- src/openvpn/socket.h | 98 ++++++++++---------- src/openvpn/socks.c | 8 +- src/openvpn/socks.h | 6 +- src/openvpn/ssl.c | 88 +++++++++--------- src/openvpn/ssl.h | 16 ++--
 src/openvpn/ssl_backend.h        |    8 +-
 src/openvpn/ssl_common.h         |   26 +++---
 src/openvpn/ssl_openssl.c        |   12 +-
src/openvpn/ssl_polarssl.c | 8 +- src/openvpn/ssl_polarssl.h | 2 +- src/openvpn/ssl_verify.c | 26 +++--- src/openvpn/ssl_verify.h | 10 +- src/openvpn/ssl_verify_openssl.c | 4 +- src/openvpn/status.c | 12 +- src/openvpn/status.h | 10 +- src/openvpn/tun.c | 104 ++++++++++---------- src/openvpn/tun.h | 52 +++++----- src/openvpn/win32.c | 28 +++--- src/openvpn/win32.h | 26 +++--- src/plugins/defer/simple.c | 9 +--
Quoted 1 line
 107 files changed, 1208 insertions(+), 1213 deletions(-)

Quoted 8 lines

Well,
As usual,
Mailing list limitation for large/complex patches inhibit proper communication.
Patch can be found/pulled from github[1].
[1] https://github.com/alonbl/openvpn/commit/bec27e73f534e97c3d24d63680a0dfe6824aef2a

David,

Your opinion regarding this one?

Alon.

David Sommerseth

2012-03-28 09:45 UTC #9 lore raw

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoted 102 lines · Alon Bar-Lev

Quoted 100 lines · Alon Bar-Lev

Quoted 89 lines · Alon Bar-Lev

PPC architectures and conflict with stdbool.h.

basic.h defines a type 'bool' that conflicts with the altivec keyword bool which has to be fixed upstream, see bugs[1][2].

[1] https://bugs.gentoo.org/show_bug.cgi?id=293840 [2]
https://bugs.gentoo.org/show_bug.cgi?id=297854

Signed-off-by Alon Bar-Lev <[email protected]...> ---

src/openvpn/basic.h | 6 +- src/openvpn/buffer.c

|   38 ++++---- src/openvpn/buffer.h             |   80
++++++++-------- src/openvpn/clinat.c             |    4 +- 
src/openvpn/console.c            |   22 ++-- src/openvpn/console.h
|    4 +- src/openvpn/crypto.c             |   34 ++++---- 
src/openvpn/crypto.h             |   30 +++--- 
src/openvpn/crypto_backend.h     |    2 +- 
src/openvpn/crypto_openssl.c     |    6 +- 
src/openvpn/crypto_polarssl.c    |    4 +- src/openvpn/error.c
|   24 +++--- src/openvpn/error.h              |   16 ++-- 
src/openvpn/event.c              |   16 ++-- src/openvpn/fdmisc.c
|    4 +- src/openvpn/fdmisc.h             |    4 +- 
src/openvpn/forward.c            |   10 +- src/openvpn/forward.h
|    4 +- src/openvpn/fragment.c           |    2 +- 
src/openvpn/fragment.h           |    8 +- src/openvpn/gremlin.c
|    8 +- src/openvpn/gremlin.h            |    2 +- 
src/openvpn/helper.c             |    2 +- src/openvpn/init.c
|   70 +++++++------- src/openvpn/init.h               |   16
++-- src/openvpn/interval.c           |    4 +- 
src/openvpn/interval.h           |   12 +- src/openvpn/list.c
|   14 ++-- src/openvpn/list.h               |   16 ++-- 
src/openvpn/lzo.c                |    8 +- src/openvpn/lzo.h
|    6 +- src/openvpn/manage.c             |   86
+++++++++--------- src/openvpn/manage.h             |   56
++++++------ src/openvpn/mbuf.c               |    4 +- 
src/openvpn/mbuf.h               |    6 +- src/openvpn/misc.c
|   54 ++++++------ src/openvpn/misc.h               |   30
+++--- src/openvpn/mroute.c             |   24 +++--- 
src/openvpn/mroute.h             |   10 +- src/openvpn/mtcp.c
|   18 ++-- src/openvpn/mtcp.h               |    2 +- 
src/openvpn/mtu.c                |    6 +- src/openvpn/mtu.h
|    8 +- src/openvpn/multi.c              |   60 ++++++------ 
src/openvpn/multi.h              |   54 ++++++------ 
src/openvpn/ntlm.c               |    2 +- src/openvpn/occ.c
|    2 +- src/openvpn/occ.h                |    2 +- 
src/openvpn/openvpn.c            |    2 +- src/openvpn/openvpn.h
|   48 +++++----- src/openvpn/options.c            |   84
+++++++++--------- src/openvpn/options.h            |  190
+++++++++++++++++++------------------- src/openvpn/otime.c
|    6 +- src/openvpn/otime.h              |   18 ++-- 
src/openvpn/packet_id.c          |   22 ++-- 
src/openvpn/packet_id.h          |   22 ++-- 
src/openvpn/pf-inline.h          |   10 +- src/openvpn/pf.c
|   32 +++--- src/openvpn/pf.h                 |   14 ++-- 
src/openvpn/ping.h               |    2 +- src/openvpn/pkcs11.c
|   22 ++-- src/openvpn/pkcs11.h             |   16 ++-- 
src/openvpn/platform.c           |   16 ++-- 
src/openvpn/platform.h           |   12 +- src/openvpn/plugin.c
|   16 ++-- src/openvpn/plugin.h             |   12 +- 
src/openvpn/pool.c               |   22 ++-- src/openvpn/pool.h
|   18 ++-- src/openvpn/proto.c              |    2 +- 
src/openvpn/proto.h              |    2 +- src/openvpn/proxy.c
|   26 +++--- src/openvpn/proxy.h              |    8 +- 
src/openvpn/ps.c                 |   12 +- src/openvpn/ps.h
|    2 +- src/openvpn/push.c               |   26 +++--- 
src/openvpn/push.h               |    8 +- src/openvpn/pushlist.h
|    2 +- src/openvpn/reliable.c           |   36 ++++---- 
src/openvpn/reliable.h           |   32 +++--- src/openvpn/route.c
|   68 +++++++------- src/openvpn/route.h              |   32
+++--- src/openvpn/session_id.h         |   10 +- 
src/openvpn/shaper.c             |    4 +- src/openvpn/shaper.h
|    4 +- src/openvpn/sig.c                |   10 +- 
src/openvpn/sig.h                |    6 +- src/openvpn/socket.c
|   94 ++++++++++---------- src/openvpn/socket.h             |
98 ++++++++++---------- src/openvpn/socks.c              |    8
+- src/openvpn/socks.h              |    6 +- src/openvpn/ssl.c
|   88 +++++++++--------- src/openvpn/ssl.h                |   16
++-- src/openvpn/ssl_backend.h        |    8 +- 
src/openvpn/ssl_common.h         |   26 +++--- 
src/openvpn/ssl_openssl.c        |   12 +- 
src/openvpn/ssl_polarssl.c       |    8 +- 
src/openvpn/ssl_polarssl.h       |    2 +- 
src/openvpn/ssl_verify.c         |   26 +++--- 
src/openvpn/ssl_verify.h         |   10 +- 
src/openvpn/ssl_verify_openssl.c |    4 +- src/openvpn/status.c
|   12 +- src/openvpn/status.h             |   10 +- 
src/openvpn/tun.c                |  104 ++++++++++---------- 
src/openvpn/tun.h                |   52 +++++----- 
src/openvpn/win32.c              |   28 +++--- src/openvpn/win32.h
|   26 +++--- src/plugins/defer/simple.c       |    9 +-- 107

Quoted 1 line

files changed, 1208 insertions(+), 1213 deletions(-)

Well,

As usual, Mailing list limitation for large/complex patches inhibit proper communication.

Patch can be found/pulled from github[1].

Quoted 7 lines

Quoted 4 lines
[1]
https://github.com/alonbl/openvpn/commit/bec27e73f534e97c3d24d63680a0dfe6824aef2a
David,
Your opinion regarding this one?

I'm in the process of reviewing all bool usage in OpenVPN. I've gone through ~60% of the source code where bool variables are used.

The reason for that review is that I'm not convinced adding our own boolean type is the way to go. I'd much rather see us using stdbool.h and keep using the bool for pure boolean purposes. Where flags needs to be used, it makes much more sense to either use a uint type, or maybe even typedef a flag_t type.

So far I've only seen bool being abused for flags in setenv_sockaddr(), setenv_in_addr_t() and setenv_link_socket_actual().

So bottom line is, please hold a bit more until I manage to complete reviewing the boolean usage properly.

kind regards,

David Sommerseth

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk9y3Z0ACgkQDC186MBRfrr7nwCgjdjh6qKk3M0XRIMTTAdugCa7 LicAn3A8lqKEEpIxtGdAd9fj5kwXa99P
=uAB7
-----END PGP SIGNATURE-----

Alon Bar-Lev

author

2012-03-28 09:48 UTC #10 lore raw

Quoted 5 lines · David Sommerseth

The reason for that review is that I'm not convinced adding our own boolean type is the way to go. I'd much rather see us using stdbool.h and keep using the bool for pure boolean purposes. Where flags needs to be used, it makes much more sense to either use a uint type, or maybe even typedef a flag_t type.

I usually try to avoid as much as possible the stdbool in C. It does not worth the effort, and people tend to misuse it in various of ways, for example storing structure with bool as a member. As bool is not properly defined, I recommend of own bool type that will behave the same in all platforms.

Alon.

Alon Bar-Lev

author

2012-04-26 22:27 UTC #11 lore raw

PATCH 1/1 build: use stdbool.h if available

If stdbool.h is available use it, otherwise create emulation.

basic.h defines a type 'bool' that conflicts with the altivec keyword bool which has to be fixed upstream, see bugs[1][2].

[1] https://bugs.gentoo.org/show_bug.cgi?id=293840
[2] https://bugs.gentoo.org/show_bug.cgi?id=297854

Signed-off-by Alon Bar-Lev <[email protected]...>

---
 configure.ac                |    2 +-
 src/compat/compat-stdbool.h |   12 ++++++++++++
 src/openvpn/basic.h         |   13 -------------
 src/openvpn/syshead.h       |    1 +
 4 files changed, 14 insertions(+), 14 deletions(-)
 create mode 100644 src/compat/compat-stdbool.h

configure.ac +1 -1

index 8ac5ac4..1a284c7 100644
@@ -354,7 +354,7 @@ AX_EMPTY_ARRAY
 AC_CHECK_SIZEOF([unsigned int])
 AC_CHECK_SIZEOF([unsigned long])
 AC_CHECK_HEADERS([ \
-	stdio.h stdarg.h limits.h \
+	stdio.h stdarg.h stdbool.h limits.h \
 	time.h errno.h fcntl.h io.h direct.h \
 	ctype.h sys/types.h sys/socket.h \
 	signal.h unistd.h dlfcn.h \

src/compat/compat-stdbool.h new file +12 -0

new file mode 100644
index 0000000..9941218
@@ -0,0 +1,12 @@
+#ifndef __COMPAT_STDBOOL_H
+#define __COMPAT_STDBOOL_H
+
+#ifdef HAVE_STDBOOL_H
+#include <stdbool.h>
+#else
+typedef int bool;
+#define false 0
+#define true 1
+#endif
+
+#endif

src/openvpn/basic.h +0 ~~-13~~

index 7c13e22..298cf10 100644
@@ -25,19 +25,6 @@
 #ifndef BASIC_H
 #define BASIC_H
 
-/* bool definitions */
-#ifndef bool
-#define bool int
-#endif
-
-#ifndef true
-#define true 1
-#endif
-
-#ifndef false
-#define false 0
-#endif
-
 #define BOOL_CAST(x) ((x) ? (true) : (false))
 
 /* size of an array */

src/openvpn/syshead.h +1 -0

index 0595b67..8bc4a0f 100644
@@ -26,6 +26,7 @@
 #define SYSHEAD_H
 
 #include "compat.h"
+#include "compat-stdbool.h"
 
 /* branch prediction hints */
 #if defined(__GNUC__)

Signature · 1 line

1.7.3.4

Alon Bar-Lev

author

2012-04-26 22:28 UTC #12 lore raw

Re: [Openvpn-devel] [PATCH] build: use stdbool.h if available

Quoted 9 lines · Alon Bar-Lev

If stdbool.h is available use it, otherwise create emulation.
basic.h defines a type 'bool' that conflicts with the altivec keyword bool which has to be fixed upstream, see bugs[1][2].
[1] https://bugs.gentoo.org/show_bug.cgi?id=293840
[2] https://bugs.gentoo.org/show_bug.cgi?id=297854

Quoted 8 lines

Signed-off-by Alon Bar-Lev <[email protected]...>
---
configure.ac | 2 +-
src/compat/compat-stdbool.h | 12 ++++++++++++
src/openvpn/basic.h | 13 -------------
src/openvpn/syshead.h | 1 +
 4 files changed, 14 insertions(+), 14 deletions(-)
create mode 100644 src/compat/compat-stdbool.h

Hello,

I took the easy approach.
I think this is the simplest it can be.

Regards,
Alon Bar-Lev.

Gert Doering

2012-04-28 08:05 UTC #13 lore raw

Re: [Openvpn-devel] [PATCH] build: use stdbool.h if available

Hi,

Quoted 2 lines · Alon Bar-Lev

Quoted 1 line · Alon Bar-Lev
If stdbool.h is available use it, otherwise create emulation.

[..]

Quoted 2 lines

I took the easy approach.
I think this is the simplest it can be.

Even if David has already committed it - thanks, this looks good.

gert

Signature · 4 lines

USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...

Alon Bar-Lev

author

2012-04-27 12:09 UTC #14 lore raw

PATCH 1/1 build: use stdbool.h if available

In reply to Alon Bar-Lev

If stdbool.h is available use it, otherwise create emulation.

basic.h defines a type 'bool' that conflicts with the altivec keyword bool which has to be fixed upstream, see bugs[1][2].

[1] https://bugs.gentoo.org/show_bug.cgi?id=293840
[2] https://bugs.gentoo.org/show_bug.cgi?id=297854

Signed-off-by Alon Bar-Lev <[email protected]...>

---
 configure.ac                |    2 +-
 src/compat/Makefile.am      |    1 +
 src/compat/compat-stdbool.h |   12 ++++++++++++
 src/openvpn/basic.h         |   13 -------------
 src/openvpn/syshead.h       |    1 +
 5 files changed, 15 insertions(+), 14 deletions(-)
 create mode 100644 src/compat/compat-stdbool.h

configure.ac +1 -1

index 8ac5ac4..1a284c7 100644
@@ -354,7 +354,7 @@ AX_EMPTY_ARRAY
 AC_CHECK_SIZEOF([unsigned int])
 AC_CHECK_SIZEOF([unsigned long])
 AC_CHECK_HEADERS([ \
-	stdio.h stdarg.h limits.h \
+	stdio.h stdarg.h stdbool.h limits.h \
 	time.h errno.h fcntl.h io.h direct.h \
 	ctype.h sys/types.h sys/socket.h \
 	signal.h unistd.h dlfcn.h \

src/compat/Makefile.am +1 -0

index 5ee35f7..7ad4452 100644
@@ -20,6 +20,7 @@ noinst_LTLIBRARIES = libcompat.la
 
 libcompat_la_SOURCES = \
 	compat.h \
+	compat-stdbool.h \
 	compat-dirname.c \
 	compat-basename.c \
 	compat-gettimeofday.c \

src/compat/compat-stdbool.h new file +12 -0

new file mode 100644
index 0000000..9941218
@@ -0,0 +1,12 @@
+#ifndef __COMPAT_STDBOOL_H
+#define __COMPAT_STDBOOL_H
+
+#ifdef HAVE_STDBOOL_H
+#include <stdbool.h>
+#else
+typedef int bool;
+#define false 0
+#define true 1
+#endif
+
+#endif

src/openvpn/basic.h +0 ~~-13~~

index 7c13e22..298cf10 100644
@@ -25,19 +25,6 @@
 #ifndef BASIC_H
 #define BASIC_H
 
-/* bool definitions */
-#ifndef bool
-#define bool int
-#endif
-
-#ifndef true
-#define true 1
-#endif
-
-#ifndef false
-#define false 0
-#endif
-
 #define BOOL_CAST(x) ((x) ? (true) : (false))
 
 /* size of an array */

src/openvpn/syshead.h +1 -0

index 0595b67..8bc4a0f 100644
@@ -26,6 +26,7 @@
 #define SYSHEAD_H
 
 #include "compat.h"
+#include "compat-stdbool.h"
 
 /* branch prediction hints */
 #if defined(__GNUC__)

Signature · 1 line

1.7.3.4

Alon Bar-Lev

author

2012-04-27 12:10 UTC #15 lore raw

Re: [Openvpn-devel] [PATCH] build: use stdbool.h if available

src/compat/Makefile.am noinst_LTLIBRARIES = libcompat.la · line 26

Quoted 10 lines · Alon Bar-Lev

src/compat/Makefile.am +1 -0

@@ -20,6 +20,7 @@ noinst_LTLIBRARIES = libcompat.la
 
  libcompat_la_SOURCES = \
        compat.h \
+       compat-stdbool.h \
        compat-dirname.c \
        compat-basename.c \
        compat-gettimeofday.c \

The above hunk was missing.

Alon.

David Sommerseth

2012-04-27 22:22 UTC #16 lore raw

Re: [Openvpn-devel] [PATCH] build: use stdbool.h if available

In reply to Alon Bar-Lev

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoted 8 lines · Alon Bar-Lev

If stdbool.h is available use it, otherwise create emulation.
basic.h defines a type 'bool' that conflicts with the altivec keyword bool which has to be fixed upstream, see bugs[1][2].
[1] https://bugs.gentoo.org/show_bug.cgi?id=293840 [2]
https://bugs.gentoo.org/show_bug.cgi?id=297854

Quoted 8 lines

Signed-off-by Alon Bar-Lev <[email protected]...> ---
configure.ac | 2 +- src/compat/Makefile.am |
1 + src/compat/compat-stdbool.h | 12 ++++++++++++
src/openvpn/basic.h | 13 -------------
src/openvpn/syshead.h | 1 + 5 files changed, 15
insertions(+), 14 deletions(-) create mode 100644
src/compat/compat-stdbool.h

ACK. Applied to master branch on -testing and -stable trees.

commit 4029971240b6274b9b30e76ff74c7f689d7d9750

Author: Alon Bar-Lev <[email protected]...>
Date:   Fri Apr 27 15:09:15 2012 +0300

    build: use stdbool.h if available

    Signed-off-by: Alon Bar-Lev <[email protected]...>
    Acked-by: David Sommerseth <[email protected]...>
    Message-Id: [email protected]...
    URL: http://article.gmane.org/gmane.network.openvpn.devel/6363
    Signed-off-by: David Sommerseth <[email protected]...>

kind regards,

David Sommerseth
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk+bHB0ACgkQDC186MBRfrrK5gCgnGCVKFuPrqdWw2gRcqb89StD tqEAnjkgJfTNK3chQiVDjc1y0OHxa43H
=J1ML
-----END PGP SIGNATURE-----