CMakeLists and Clang sanitization options for an ncurses program

Question

I would like some feedback on a CMakeLists.txt file I created for compiling my project. I have pasted the CMakeLists as well as my source code below. One thing I would specifically appreciate feedback on the sanitization options I have enabled. Are there more I should enable, and/or should I should reduce? I know -fsanitize=address, -fsanitize=thread, and -fsanitize=memory groups can't be used with others (according to the clang documentation). Would one of the other groups be better preferred to use on a first-pass rather than the one I chose (address)?

Also - the blob feature I am using, I have based on a StackOverflow answer I read - I understand that this doesn't detect new C source files and I'm fine with that, but besides that subtle detail is this an okay practice to follow?

CMakeLists.txt

cmake_minimum_required(VERSION 3.13)
project(FirstProject C)
find_package(Curses REQUIRED)
include_directories(${CURSES_INCLUDE_DIR})
set(CMAKE_C_COMPILER clang)
set(CMAKE_C_STANDARD 99)
set (CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Weverything -fsanitize=undefined,integer,implicit-conversion,nullability,address,leak,cfi -flto -fvisibility=default")
FILE(GLOB Sources *.c)
add_executable(${CMAKE_PROJECT_NAME} ${Sources})
target_link_libraries(${CMAKE_PROJECT_NAME} ${CURSES_LIBRARIES})

main.c (Code comes from here)

#include <ctype.h>
#include <errno.h>
#include <inttypes.h>
#include <limits.h>
#include <ncurses.h>
#include <stdint.h>
#include <stdlib.h>
#include <string.h>
// Generous estimate of the maximum number of digits
// https://stackoverflow.com/a/10536254
#define ULL_DIGITS (3 * sizeof(unsigned long long))
#define ERR_MSG_MAX_LENGTH 32
#define NUL '0円'
#define NUL_SIZE 1
int ask_ull(unsigned long long *result, const char *prompt);
/**
 * Prints a prompt then reads an unsigned long long, using ncurses.
 * Returns 0 on success. Returns errno on failure, which is set to
 * ERANGE, EDOM, or EIO.
 */
int ask_ull(unsigned long long *result, const char *prompt) {
 char buf[ULL_DIGITS + NUL_SIZE];
 char *endptr;
 printw("%s", prompt);
 getnstr(buf, ULL_DIGITS);
 *result = strtoull(buf, &endptr, 10);
 if (errno == ERANGE) {
 // Overflow or underflow
 return errno;
 }
 if (endptr == buf || strchr(buf, '-')) {
 // Unsuccessful conversion
 errno = EDOM;
 return errno;
 }
 while (isspace(*endptr)) endptr++;
 if (*endptr) {
 // Trailing junk
 errno = EIO;
 return errno;
 }
 errno = 0;
 return errno;
}
int main(void) {
 unsigned long long height, width, length;
 height = width = length = 0;
 char errmsg[ERR_MSG_MAX_LENGTH];
 errmsg[0] = NUL;
 initscr();
 printw("--- Volume Calculator --\n");
 if (!ask_ull(&length, "Enter length: ")) {
 sscanf(errmsg, "%s", "Unable to scan length");
 }
 if (!ask_ull(&width, "Enter width: ")) {
 sscanf(errmsg, "%s", "Unable to scan width");
 }
 if (!ask_ull(&height, "Enter height: ")) {
 sscanf(errmsg, "%s", "Unable to scan height");
 }
 if (errmsg[0] != NUL) {
 refresh();
 endwin();
 perror(errmsg);
 return errno;
 }
 unsigned long long volume = length * width * height;
 printw("Volume: %llu", volume);
 refresh();
 getch();
 endwin();
}

Answer 1

I am not familiar with the Clang sanitization command-line options so I can't give any feedback about those, but regarding the CMake code I would suggest the following CMakeLists.txt file

cmake_minimum_required(VERSION 3.13)
project(FirstProject C)
find_package(Curses REQUIRED)
add_executable(${CMAKE_PROJECT_NAME} main.c)
target_include_directories(${CMAKE_PROJECT_NAME} PRIVATE ${CURSES_INCLUDE_DIR})
target_link_libraries(${CMAKE_PROJECT_NAME} PRIVATE ${CURSES_LIBRARIES})
if(NOT CMAKE_C_COMPILER_ID STREQUAL "Clang")
 message(WARNING "Use the Clang compiler instead. "
 "FirstProject officially supports Clang "
 "(although other compilers might work).") 
endif() 
target_compile_features(${CMAKE_PROJECT_NAME} PRIVATE c_std_99)
target_compile_options(${CMAKE_PROJECT_NAME} PRIVATE
 $<$<C_COMPILER_ID:Clang>:
 -Weverything
 -fsanitize=undefined,integer,implicit-conversion,nullability,address,leak,cfi
 -flto
 -fvisibility=default>)
target_link_options(${CMAKE_PROJECT_NAME} PRIVATE
 $<$<C_COMPILER_ID:Clang>:
 -fsanitize=undefined,integer,implicit-conversion,nullability,address,leak,cfi
 -flto>)

Some comments

Avoid using FILE(GLOB) to specify source code files

Avoid using FILE(GLOB), instead specify the source code files explicitly either by

add_executable(${CMAKE_PROJECT_NAME} main.c)

or

add_executable(${CMAKE_PROJECT_NAME})
target_sources(${CMAKE_PROJECT_NAME} PRIVATE main.c)

Śee also https://stackoverflow.com/questions/32411963/why-is-cmake-file-glob-evil

Use target_* commands

Avoid

• using include_directories()
• setting CMAKE_C_FLAGS directly

Instead use

• target_compile_features()
• target_compile_options()
• target_include_directories()
• target_link_libraries()
• target_link_options()

The FindCurses module does not yet support imported targets as of today (2 January 2019, CMake 3.13.2) so it needs to be used in the old style

target_include_directories(${CMAKE_PROJECT_NAME} PRIVATE ${CURSES_INCLUDE_DIR})
target_link_libraries(${CMAKE_PROJECT_NAME} PRIVATE ${CURSES_LIBRARIES})

In the future (when support has been added to CMake for imported targets in FindCurses) the two lines should be replaced by the line:

target_link_libraries(${CMAKE_PROJECT_NAME} PRIVATE Curses::Curses)

Instead of setting the CMake variable CMAKE_C_STANDARD

set(CMAKE_C_STANDARD 99)

is good practice to use target_compile_features() instead

 target_compile_features(${CMAKE_PROJECT_NAME} PRIVATE c_std_99)

In this very case it makes no practical difference but for a C++ header-only library, such compile features could be specified in the INTERFACE

 add_library(myheaderonly INTERFACE)
 target_compile_features(headeronlylib INTERFACE cxx_std_11)

to provide usage requirements for consumers of the library (see also https://cmake.org/cmake/help/latest/manual/cmake-buildsystem.7.html)

The target_link_options() line was added to be able to build the executable. (I am not sure it is correct).

Use generator expressions

The generator expression

$<$<C_COMPILER_ID:Clang>:-Weverything -fsanitize=undefined,integer,implicit-conversion,nullability,address,leak,cfi -flto -fvisibility=default>

is expanded to

-Weverything -fsanitize=undefined,integer,implicit-conversion,nullability,address,leak,cfi -flto -fvisibility=default

when the Clang compiler is used, but for other compilers it is expanded to nothing.

Avoid setting CMAKE_C_COMPILER

Instead of setting the CMake variable CMAKE_C_COMPILER, give a WARNING or a FATAL_ERROR whenever a non-supported C compiler is used.

if(NOT CMAKE_C_COMPILER_ID STREQUAL "Clang")
 message(WARNING "Use the Clang compiler instead. FirstProject officially supports Clang (although other compilers might work).") 
endif() 

(WARNING could be replaced by FATAL_ERROR to prevent the use of any other C compiler than Clang)

To compile the project, specify the C compiler with the environment variable CC

mkdir /tmp/build
cd /tmp/build
CC=clang cmake -G Ninja ~/FirstProject
ninja -v

Use the ninja command-line flag -v if you want to see the actual commands being run.

• Faraz: I appreciate the thorough answer. The last segment though, any particular advantage of using ninja over traditional make? Will it make a difference in compile-time speed?
• Erik Sjölund: Probably very little. For me the main advantages of ninja is that you don't need to specify the number of threads (e.g. make -j) and that the software is available for Linux, Mac and Windows.