Skip to content

Commit

Permalink
build: speed up and extend picky compiler options
Browse files Browse the repository at this point in the history 
Implement picky warnings with clang in autotools. Extend picky gcc
warnings, sync them between build tools and compilers and greatly
speed up detection in CMake.

- autotools: enable clang compiler warnings with `--enable-debug`.

- autotools: enable more gcc compiler warnings with `--enable-debug`.

- autotools/cmake: sync compiler warning options between gcc and clang.

- sync compiler warning options between autotools and cmake.

- cmake: reduce option-checks to speed up the detection phase.
  Bring them down to 3 (from 35). Leaving some checks to keep the
  CMake logic alive and for an easy way to add new options.

  clang 3.0 (2011-11-29) and gcc 2.95 (1999-07-31) now required.

- autotools logic copied from curl, with these differences:

  - delete `-Wimplicit-fallthrough=4` due to a false positive.

  - reduce `-Wformat-truncation=2` to `1` due to a false positive.

  - simplify MinGW detection for `-Wno-pedantic-ms-format`.

- cmake: show enabled picky compiler options (like autotools).

- cmake: do compile `tests/simple.c` and `tests/ssh2.c`.

- fix new compiler warnings.

- `tests/CMakeLists.txt`: fix indentation.

Original source of autotools logic:
- https://github.com/curl/curl/blob/a8fbdb461cecbfe1ac6ecc5d8f6cf181e1507da8/acinclude.m4
- https://github.com/curl/curl/blob/a8fbdb461cecbfe1ac6ecc5d8f6cf181e1507da8/m4/curl-compilers.m4

Notice that the autotools implementation considers Apple clang as
legacy clang 3.7. CMake detection works more accurately, at the same
time more error-prone and difficult to update due to the sparsely
documented nature of Apple clang option evolution.

Closes #952
  • Loading branch information
@vszakats
vszakats committed Apr 13, 2023
1 parent 224fffb commit ec0feae
Show file tree
Hide file tree
Showing 21 changed files with 740 additions and 196 deletions.
398 changes: 394 additions & 4 deletions acinclude.m4
View file

Large diffs are not rendered by default.

170 changes: 160 additions & 10 deletions cmake/max_warnings.cmake
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ if(MSVC)
endif()
endif()
elseif(CMAKE_COMPILER_IS_GNUCC OR CMAKE_COMPILER_IS_GNUCXX OR CMAKE_C_COMPILER_ID MATCHES "Clang")

# https://clang.llvm.org/docs/DiagnosticsReference.html
# https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html

if(NOT CMAKE_CXX_FLAGS MATCHES "-Wall")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall")
endif()
Expand All @@ -36,23 +40,169 @@ elseif(CMAKE_COMPILER_IS_GNUCC OR CMAKE_COMPILER_IS_GNUCXX OR CMAKE_C_COMPILER_I
endif()

if(PICKY_COMPILER)
foreach(_CCOPT -pedantic -W -Wpointer-arith -Wwrite-strings -Wunused -Wshadow -Winline -Wnested-externs -Wmissing-declarations -Wmissing-prototypes -Wfloat-equal -Wsign-compare -Wundef -Wendif-labels -Wstrict-prototypes -Wdeclaration-after-statement -Wstrict-aliasing=3 -Wcast-align -Wtype-limits -Wold-style-declaration -Wmissing-parameter-type -Wempty-body -Wclobbered -Wignored-qualifiers -Wconversion -Wvla -Wdouble-promotion -Wenum-conversion -Warith-conversion)

# WPICKY_ENABLE = Options we want to enable as-is.
# WPICKY_DETECT = Options we want to test first and enable if available.

# Prefer the -Wextra alias with clang.
if(CMAKE_C_COMPILER_ID MATCHES "Clang")
set(WPICKY_ENABLE "-Wextra")
else()
set(WPICKY_ENABLE "-W")
endif()

list(APPEND WPICKY_ENABLE
-pedantic
)

# ----------------------------------
# Add new options here, if in doubt:
# ----------------------------------
set(WPICKY_DETECT
)

# Assume these options always exist with both clang and gcc.
# Require clang 3.0 / gcc 2.95 or later.
list(APPEND WPICKY_ENABLE
-Wconversion # clang 3.0 gcc 2.95
-Winline # clang 1.0 gcc 1.0
-Wmissing-declarations # clang 1.0 gcc 2.7
-Wmissing-prototypes # clang 1.0 gcc 1.0
-Wnested-externs # clang 1.0 gcc 1.0
-Wno-long-long # clang 1.0 gcc 2.95
-Wno-multichar # clang 1.0 gcc 2.95
-Wpointer-arith # clang 1.0 gcc 1.0
-Wshadow # clang 1.0 gcc 2.95
-Wsign-compare # clang 1.0 gcc 2.95
-Wundef # clang 1.0 gcc 2.95
-Wunused # clang 1.1 gcc 2.95
-Wwrite-strings # clang 1.0 gcc 1.0
)

# Always enable with clang, version dependent with gcc
set(WPICKY_COMMON_OLD
-Wcast-align # clang 1.0 gcc 4.2
-Wdeclaration-after-statement # clang 1.0 gcc 3.4
-Wempty-body # clang 3.0 gcc 4.3
-Wendif-labels # clang 1.0 gcc 3.3
-Wfloat-equal # clang 1.0 gcc 2.96 (3.0)
-Wignored-qualifiers # clang 3.0 gcc 4.3
-Wno-format-nonliteral # clang 1.0 gcc 2.96 (3.0)
-Wno-sign-conversion # clang 3.0 gcc 4.3
-Wno-system-headers # clang 1.0 gcc 3.0
-Wstrict-prototypes # clang 1.0 gcc 3.3
-Wtype-limits # clang 3.0 gcc 4.3
-Wvla # clang 2.8 gcc 4.3
)

set(WPICKY_COMMON
-Wdouble-promotion # clang 3.6 gcc 4.6 appleclang 6.3
-Wenum-conversion # clang 3.2 gcc 10.0 appleclang 4.6 g++ 11.0
-Wunused-const-variable # clang 3.4 gcc 6.0 appleclang 5.1
)

if(CMAKE_C_COMPILER_ID MATCHES "Clang")
list(APPEND WPICKY_ENABLE
${WPICKY_COMMON_OLD}
-Wshift-sign-overflow # clang 2.9
-Wshorten-64-to-32 # clang 1.0
)
# Enable based on compiler version
if((CMAKE_C_COMPILER_ID STREQUAL "Clang" AND NOT CMAKE_C_COMPILER_VERSION VERSION_LESS 3.6) OR
(CMAKE_C_COMPILER_ID STREQUAL "AppleClang" AND NOT CMAKE_C_COMPILER_VERSION VERSION_LESS 6.3))
list(APPEND WPICKY_ENABLE
${WPICKY_COMMON}
)
endif()
if((CMAKE_C_COMPILER_ID STREQUAL "Clang" AND NOT CMAKE_C_COMPILER_VERSION VERSION_LESS 3.9) OR
(CMAKE_C_COMPILER_ID STREQUAL "AppleClang" AND NOT CMAKE_C_COMPILER_VERSION VERSION_LESS 8.3))
list(APPEND WPICKY_ENABLE
-Wcomma # clang 3.9 appleclang 8.3
-Wmissing-variable-declarations # clang 3.2 appleclang 4.6
)
endif()
if((CMAKE_C_COMPILER_ID STREQUAL "Clang" AND NOT CMAKE_C_COMPILER_VERSION VERSION_LESS 7.0) OR
(CMAKE_C_COMPILER_ID STREQUAL "AppleClang" AND NOT CMAKE_C_COMPILER_VERSION VERSION_LESS 10.3))
list(APPEND WPICKY_ENABLE
-Wassign-enum # clang 7.0 appleclang 10.3
-Wextra-semi-stmt # clang 7.0 appleclang 10.3
)
endif()
else() # gcc
list(APPEND WPICKY_DETECT
${WPICKY_COMMON}
)
# Enable based on compiler version
if(NOT CMAKE_C_COMPILER_VERSION VERSION_LESS 4.3)
list(APPEND WPICKY_ENABLE
${WPICKY_COMMON_OLD}
-Wmissing-parameter-type # gcc 4.3
-Wold-style-declaration # gcc 4.3
-Wstrict-aliasing=3 # gcc 4.0
)
endif()
if(NOT CMAKE_C_COMPILER_VERSION VERSION_LESS 4.5 AND MINGW)
list(APPEND WPICKY_ENABLE
-Wno-pedantic-ms-format # gcc 4.5 (mingw-only)
)
endif()
if(NOT CMAKE_C_COMPILER_VERSION VERSION_LESS 4.8)
list(APPEND WPICKY_ENABLE
-Wformat=2 # clang 3.0 gcc 4.8 (clang part-default, enabling it fully causes -Wformat-nonliteral warnings)
)
endif()
if(NOT CMAKE_C_COMPILER_VERSION VERSION_LESS 5.0)
list(APPEND WPICKY_ENABLE
-Warray-bounds=2 -ftree-vrp # clang 3.0 gcc 5.0 (clang default: -Warray-bounds)
)
endif()
if(NOT CMAKE_C_COMPILER_VERSION VERSION_LESS 6.0)
list(APPEND WPICKY_ENABLE
-Wduplicated-cond # gcc 6.0
-Wnull-dereference # clang 3.0 gcc 6.0 (clang default)
-fdelete-null-pointer-checks
-Wshift-negative-value # clang 3.7 gcc 6.0 (clang default)
-Wshift-overflow=2 # clang 3.0 gcc 6.0 (clang default: -Wshift-overflow)
)
endif()
if(NOT CMAKE_C_COMPILER_VERSION VERSION_LESS 7.0)
list(APPEND WPICKY_ENABLE
-Walloc-zero # gcc 7.0
-Wduplicated-branches # gcc 7.0
-Wformat-overflow=2 # gcc 7.0
-Wformat-truncation=1 # gcc 7.0
-Wrestrict # gcc 7.0
)
endif()
if(NOT CMAKE_C_COMPILER_VERSION VERSION_LESS 10.0)
list(APPEND WPICKY_ENABLE
-Warith-conversion # gcc 10.0
)
endif()
endif()

#

unset(WPICKY)

foreach(_CCOPT ${WPICKY_ENABLE})
set(WPICKY "${WPICKY} ${_CCOPT}")
endforeach()

foreach(_CCOPT ${WPICKY_DETECT})
# surprisingly, CHECK_C_COMPILER_FLAG needs a new variable to store each new
# test result in.
string(MAKE_C_IDENTIFIER "OPT${_CCOPT}" _optvarname)
check_c_compiler_flag(${_CCOPT} ${_optvarname})
if(${_optvarname})
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${_CCOPT}")
endif()
endforeach()
foreach(_CCOPT long-long multichar format-nonliteral sign-conversion system-headers pedantic-ms-format)
# GCC only warns about unknown -Wno- options if there are also other diagnostic messages,
# so test for the positive form instead
string(MAKE_C_IDENTIFIER "OPT${_CCOPT}" _optvarname)
check_c_compiler_flag("-W${_CCOPT}" ${_optvarname})
string(REPLACE "-Wno-" "-W" _CCOPT_ON "${_CCOPT}")
check_c_compiler_flag(${_CCOPT_ON} ${_optvarname})
if(${_optvarname})
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wno-${_CCOPT}")
set(WPICKY "${WPICKY} ${_CCOPT}")
endif()
endforeach()

message(STATUS "Picky compiler options:${WPICKY}")
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${WPICKY}")
endif()
endif()
4 changes: 1 addition & 3 deletions example/sftpdir_nonblock.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,9 +185,7 @@ int main(int argc, char *argv[])

/* loop until we fail */
while((rc = libssh2_sftp_readdir(sftp_handle, mem, sizeof(mem),
&attrs)) == LIBSSH2_ERROR_EAGAIN) {
;
}
&attrs)) == LIBSSH2_ERROR_EAGAIN);
if(rc > 0) {
/* rc is the length of the file name in the mem
buffer */
Expand Down
6 changes: 3 additions & 3 deletions example/tcpip-forward.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,11 @@ static const char *server_ip = "127.0.0.1";
/* resolved by the server */
static const char *remote_listenhost = "localhost";

int remote_wantport = 2222;
int remote_listenport;
static int remote_wantport = 2222;
static int remote_listenport;

static const char *local_destip = "127.0.0.1";
int local_destport = 22;
static int local_destport = 22;

enum {
AUTH_NONE = 0,
Expand Down
4 changes: 2 additions & 2 deletions example/x11.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ struct chan_X11_list {
struct chan_X11_list *next;
};

struct chan_X11_list * gp_x11_chan = NULL;
struct termios _saved_tio;
static struct chan_X11_list * gp_x11_chan = NULL;
static struct termios _saved_tio;

/*
* Utility function to remove a Node of the chained list
Expand Down
28 changes: 15 additions & 13 deletions src/agent.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,20 +128,22 @@ agent_connect_unix(LIBSSH2_AGENT *agent)
}

#define RECV_SEND_ALL(func, socket, buffer, length, flags, abstract) \
size_t finished = 0; \
do { \
size_t finished = 0; \
\
while(finished < length) { \
ssize_t rc; \
rc = func(socket, \
(char *)buffer + finished, length - finished, \
flags, abstract); \
if(rc < 0) \
return rc; \
while(finished < length) { \
ssize_t rc; \
rc = func(socket, \
(char *)buffer + finished, length - finished, \
flags, abstract); \
if(rc < 0) \
return rc; \
\
finished += rc; \
} \
finished += rc; \
} \
\
return finished;
return finished; \
} while(0)

static ssize_t _send_all(LIBSSH2_SEND_FUNC(func), libssh2_socket_t socket,
const void *buffer, size_t length,
Expand Down Expand Up @@ -242,7 +244,7 @@ agent_disconnect_unix(LIBSSH2_AGENT *agent)
return LIBSSH2_ERROR_NONE;
}

struct agent_ops agent_ops_unix = {
static struct agent_ops agent_ops_unix = {
agent_connect_unix,
agent_transact_unix,
agent_disconnect_unix
Expand Down Expand Up @@ -347,7 +349,7 @@ agent_disconnect_pageant(LIBSSH2_AGENT *agent)
return 0;
}

struct agent_ops agent_ops_pageant = {
static struct agent_ops agent_ops_pageant = {
agent_connect_pageant,
agent_transact_pageant,
agent_disconnect_pageant
Expand Down
6 changes: 3 additions & 3 deletions src/agent.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,9 @@ struct agent_publickey {
};

struct agent_ops {
agent_connect_func connect;
agent_transact_func transact;
agent_disconnect_func disconnect;
const agent_connect_func connect;
const agent_transact_func transact;
const agent_disconnect_func disconnect;
};

struct _LIBSSH2_AGENT
Expand Down
2 changes: 1 addition & 1 deletion src/comp.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ comp_method_zlib_decomp(LIBSSH2_SESSION * session,
/* If strm is null, then we have not yet been initialized. */
if(strm == NULL)
return _libssh2_error(session, LIBSSH2_ERROR_COMPRESS,
"decompression uninitialized");;
"decompression uninitialized");

/* In practice they never come smaller than this */
if(out_maxlen < 25)
Expand Down
4 changes: 2 additions & 2 deletions src/hostkey.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -894,7 +894,7 @@ hostkey_method_ssh_ecdsa_sig_verify(LIBSSH2_SESSION * session,


#define LIBSSH2_HOSTKEY_METHOD_EC_SIGNV_HASH(digest_type) \
{ \
do { \
unsigned char hash[SHA##digest_type##_DIGEST_LENGTH]; \
libssh2_sha##digest_type##_ctx ctx; \
int i; \
Expand All @@ -907,7 +907,7 @@ hostkey_method_ssh_ecdsa_sig_verify(LIBSSH2_SESSION * session,
ret = _libssh2_ecdsa_sign(session, ec_ctx, hash, \
SHA##digest_type##_DIGEST_LENGTH, \
signature, signature_len); \
}
} while(0)


/*
Expand Down

0 comments on commit ec0feae

Please sign in to comment.