Re: [PATCH V2] examples: Add ssh_server_fork example

[Raphael Kubo da Costa <rakuco@xxxxxxxxxxx>]

Audrius Butkevicius
<audrius.butkevicius@xxxxxxxxxxxxxxxx> writes:

> Patch attached, but a few points/questions:
> 1. I haven't used CMake before, but I have been told to add:
> check_include_file(libutil.h HAVE_LIBUTIL_H)
>
> to ConfigureChecks.cmake but I've noticed that there is already:
> check_library_exists(util forkpty "" HAVE_LIBUTIL)
>
> And this example is only build `if (HAVE_LIBUTIL)`, just like the old
> pty example, so I am not sure if this check_include_file is actually
> relevant, as I assume the old example was never built for FreeBSD?

It is, but with a warning:

/s/libssh/examples/samplesshd-tty.c:298:16: warning: implicit
declaration of function 'forkpty' is invalid in C99 [-Wimplicit-function-declaration]

Which makes sense, since forkpty(3) is in libutil.h, not util.h, on
FreeBSD. If you're interested in it, you could fix this in
samplesshd-tty.c now that you've added a check for libutil.h.
Conversely, your new file should check for util.h which is what's
present in other systems.

> Perhaps we should instead have:
> check_library_exists(libutil forkpty "" HAVE_ALTLIBUTIL)
>
> and build only `if (HAVE_LIBUTIL OR HAVE_ALTLIBUTIL)`?

That call does exactly the same thing as the existing one (perhaps in a
less portable way as it assumes libraries have the "lib" prefix).

The new check_include_file() call is relevant because we now detect the
right header on FreeBSD; the library is called libutil on multiple
operating systems, but the header name varies. By adding the new
check_include_file() call and checking for HAVE_LIBUTIL_H in the code
you're accouting for all the variations.

> 2. The build is currently not dependent on HAVE_PTY_H or
> HAVE_LIBUTIL_H, but I feel that it should be?

I don't see how this would make a difference. The build system just
needs to know whether libutil is present and link against it, the name
of the header file is orthogonal here (if libutil is present then it
follows that at least one of HAVE_{PTY,UTIL,LIBUTIL}_H is set anyway).