Re: [PATCH V2] examples: Add ssh_server_fork example

[Aris Adamantiadis <aris@xxxxxxxxxxxx>]

Hi,

Thanks for your comments on this libutil issue. I don't have an opinion
on this, except that maybe you shouldn't invest too much time on this
issue. The examples are there to demonstrate the use of libssh's API,
not to be of production quality.

I'd take any simple patch that removes the warning, e.g. by including
the missing include on freebsd (or HAVE_LIBUTIL_H is defined).

Aris

Le 10/02/14 00:14, Raphael Kubo da Costa a écrit :
> 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).
> 
>