Hello Christian,

On Mon, Feb 03, 2020 at 12:09:46PM +0100, Chris wrote:
> Hello everybody,
> 
> I spent some time making haproxy compile and run successfully on AIX
> 7.2 using GCC 8.3 and wanted to contribute my patch in the hope that
> it could be merged. The patch is based on the current haproxy 2.1 head
> revision. I can make one for the development branch too - but it
> should be basically identical.

Thank you for doing this work. I'm having a few questions below.

> +# AIX 7.2 and above
> +ifeq ($(TARGET),aix72-gcc)
> +  set_target_defaults = $(call default_opts, \
> +    USE_POLL USE_THREAD USE_LIBCRYPT USE_OBSOLETE_LINKER
> USE_GETADDRINFO USE_TFO)

Are you really really sure about USE_TFO ? I suspect you might have
accidently borrowed it from another line. It stands for TCP FastOpen
and is really not broadly deployed, I was only aware of Linux and
FreeBSD, but there are likely a few other ones. And the fact that it
looks OK is possibly just that it's causing a different TCP socket
option to be set on the connection, so if you're not certain we'd
rather avoid it.

> The patch implements a new TARGET called aix72-gcc and also adds 2
> CPUs (power8 and power9). Here is my proof-of-work:
(...)
> src/hlua.c: In function 'hlua_panic_ljmp':
> src/hlua.c:128:1: warning: no return statement in function returning
> non-void [-Wreturn-type]
>  static int hlua_panic_ljmp(lua_State *L) { longjmp(safe_ljmp_env, 1); }
>  ^~~~~~

Don't worry about this one, I'll handle it. I suspect that on linux
platforms the longjmp() function prototype is decorated with
__attribute__((noreturn)) which makes the compiler happy, but that's
likely not the case on any system not relying on a gcc-compatible
compiler by default.

> -bash-4.4$ ./haproxy -vv
> HA-Proxy version 2.1.2 2019/12/21 - https://haproxy.org/
> Status: stable branch - will stop receiving fixes around Q1 2021.
(...)

Looks good!

> If you have any questions feel free to ask and please keep me on CC
> for this topic!

Just two small extra requests :
  - please rebase it on top of the development branch. If you really
    need it in 2.1, just indicate it. Given that it's well isolated,
    I'm fine with having it backported.

  - please have a look at CONTRIBUTING to get guidance to write a
    subject and a commit message. It doesn't need to be very long
    but at least indicating what options you chosed to enable/disable
    and on what system you tested it will be enough if we need to
    reconsider parts of it later.

thanks!
Willy

Reply via email to