Re: [PATCH] Introduce symbol versioning

[Jakub Jelen <jjelen@xxxxxxxxxx>]

On Fri, 2018-03-09 at 07:37 -0500, Anderson Sasaki wrote:
> [...]
> 
> From 3e38ba07df7667300714771dfbe72bbd3077f582 Mon Sep 17 00:00:00
> 2001
> From: Anderson Toshiyuki Sasaki <ansasaki@xxxxxxxxxx>
> Date: Thu, 8 Mar 2018 13:15:34 +0100
> Subject: [PATCH] Introduce symbol versioning
> 
> This adds a linker map, which adds version information for exported
> symbols, and the option to compile with symbol versioning. The symbol
> versioning is enabled by default, but disabled in non UNIX like
> platforms. It can be disabled by passing "-
> DWITH_SYMBOL_VERSIONING=OFF"
> option to cmake or "-withoutsymbolversioning" to "obj/build_make.sh".

There should be two dashes, isn't it? "--withoutsymbolversioning"

> [...]
> +
> +LIBSSH_4_5_0

Why the version 4_5_0 ? This will be the first symbol release? Or is
this some convention?

> +{
> +    global:
> +        buffer_free;
> +        buffer_get;
> +        buffer_get_len;
> +        buffer_new;
> +        channel_accept_x11;
> +        channel_change_pty_size;
> +        channel_close;
> +        channel_forward_accept;
> +        channel_forward_cancel;
> +        channel_forward_listen;
> +        channel_free;
> +        channel_get_exit_status;
> +        channel_get_session;
> +        channel_is_closed;
> +        channel_is_eof;
> +        channel_is_open;
> +        channel_new;
> +        channel_open_forward;
> +        channel_open_session;
> +        channel_poll;
> +        channel_read;
> +        channel_read_buffer;
> +        channel_read_nonblocking;
> +        channel_request_env;
> +        channel_request_exec;
> +        channel_request_pty;
> +        channel_request_pty_size;
> +        channel_request_send_signal;
> +        channel_request_sftp;
> +        channel_request_shell;
> +        channel_request_subsystem;
> +        channel_request_x11;
> +        channel_select;
> +        channel_send_eof;
> +        channel_set_blocking;
> +        channel_write;
> +        channel_write_stderr;
> +        privatekey_free;
> +        privatekey_from_file;
> +        publickey_free;
> +        publickey_from_file;
> +        publickey_from_privatekey;
> +        publickey_to_string;
> [...]
> +        _ssh_log;
> [...]
> +        string_burn;
> +        string_copy;
> +        string_data;
> +        string_fill;
> +        string_free;
> +        string_from_char;
> +        string_len;
> +        string_new;
> +        string_to_char;
> +    local:
> +        *;
> +};

Should the buffer_*, channel_*, string_*, publicky_*, privatekey_*
functions and obviously-internal _ssh_log() be covered in the API? I
don't think so. I would cover only the ssh_* and sftp_*, which is to my
understanding intended API.

I don't have a lot experience with it so these things might just need
some clarification why they are done this way.

Regards,
-- 
Jakub Jelen
Software Engineer
Security Technologies
Red Hat, Inc.