Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Converge the warning-flags in Makefile and CMake builds #67

Merged
merged 3 commits into from
Aug 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ jobs:
- name: Build using cmake
env:
EXTRA_CMAKE_OPTS: -DENABLE_EXAMPLES:BOOL=ON -DENABLE_SSL:BOOL=ON
CFLAGS: -Werror
CXXFLAGS: -Werror
run: mkdir build && cd build && cmake .. && make

- name: Build using makefile
Expand Down Expand Up @@ -57,8 +55,6 @@ jobs:
- name: Build using cmake
env:
EXTRA_CMAKE_OPTS: -DENABLE_EXAMPLES:BOOL=ON -DENABLE_SSL:BOOL=ON
CFLAGS: -Werror
CXXFLAGS: -Werror
run: mkdir build && cd build && cmake3 .. && make

- name: Build using Makefile
Expand Down Expand Up @@ -102,8 +98,6 @@ jobs:
- name: Build using cmake
env:
EXTRA_CMAKE_OPTS: -DENABLE_EXAMPLES:BOOL=ON -DENABLE_SSL:BOOL=ON
CFLAGS: -Werror
CXXFLAGS: -Werror
run: mkdir build && cd build && cmake .. && make

- name: Build using Makefile
Expand Down
12 changes: 8 additions & 4 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,14 @@ OPTION(ENABLE_RDMA "Build valkey_rdma for RDMA support" OFF)
SET(CMAKE_C_STANDARD 99)
SET(CMAKE_DEBUG_POSTFIX d)

# Set target-common flags
if(NOT WIN32)
add_compile_options(-Werror -Wall -Wextra -pedantic)
add_compile_options(-Wstrict-prototypes -Wwrite-strings -Wno-missing-field-initializers)
else()
add_definitions(-D_CRT_SECURE_NO_WARNINGS -DWIN32_LEAN_AND_MEAN)
endif()

SET(valkey_sources
src/adlist.c
src/alloc.c
Expand All @@ -45,10 +53,6 @@ SET(valkey_sources
src/valkeycluster.c
src/vkutil.c)

IF(WIN32)
ADD_DEFINITIONS(-D_CRT_SECURE_NO_WARNINGS -DWIN32_LEAN_AND_MEAN)
ENDIF()

ADD_LIBRARY(valkey ${valkey_sources})
ADD_LIBRARY(valkey::valkey ALIAS valkey)
set(valkey_export_name valkey CACHE STRING "Name of the exported target")
Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ export VALKEY_TEST_CONFIG
CC := $(if $(shell command -v $(firstword $(CC)) >/dev/null 2>&1 && echo OK),$(CC),gcc)

OPTIMIZATION?=-O3
WARNINGS=-Wall -Wextra -Wstrict-prototypes -Wwrite-strings -Wno-missing-field-initializers
WARNINGS=-Wall -Wextra -pedantic -Wstrict-prototypes -Wwrite-strings -Wno-missing-field-initializers
USE_WERROR?=1
ifeq ($(USE_WERROR),1)
WARNINGS+=-Werror
Expand Down
4 changes: 1 addition & 3 deletions tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,10 @@ find_path(LIBEVENT_INCLUDES event2/event.h)
if(LIBEVENT_INCLUDES)
include_directories(${LIBEVENT_INCLUDES})
endif()

if(MSVC OR MINGW)
find_library(LIBEVENT_LIBRARY Libevent)
else()
add_compile_options(-Wall -Wextra -pedantic -Werror)
# Debug mode for tests
# Use the Debug configuration when building tests (no -DNDEBUG)
set(CMAKE_BUILD_TYPE "Debug" CACHE STRING "" FORCE)
endif()

Expand Down
2 changes: 1 addition & 1 deletion tests/client_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -1583,7 +1583,7 @@ void async_disconnect(valkeyAsyncContext *ac) {
}

/* Testcase timeout, will trigger a failure */
void timeout_cb(int fd, short event, void *arg) {
void timeout_cb(evutil_socket_t fd, short event, void *arg) {
(void) fd; (void) event; (void) arg;
printf("Timeout in async testing!\n");
exit(1);
Expand Down
2 changes: 1 addition & 1 deletion tests/clusterclient.c
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ void printReply(const valkeyReply *reply) {
void eventCallback(const valkeyClusterContext *cc, int event, void *privdata) {
(void)cc;
(void)privdata;
char *e = NULL;
const char *e;
switch (event) {
case VALKEYCLUSTER_EVENT_SLOTMAP_UPDATED:
e = "slotmap-updated";
Expand Down
4 changes: 2 additions & 2 deletions tests/clusterclient_async.c
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ int num_running = 0;
int resend_failed_cmd = 0;
int send_to_all = 0;

void sendNextCommand(int, short, void *);
void sendNextCommand(evutil_socket_t, short, void *);

void printReply(const valkeyReply *reply) {
switch (reply->type) {
Expand Down Expand Up @@ -102,7 +102,7 @@ void replyCallback(valkeyClusterAsyncContext *acc, void *r, void *privdata) {
}
}

void sendNextCommand(int fd, short kind, void *arg) {
void sendNextCommand(evutil_socket_t fd, short kind, void *arg) {
UNUSED(fd);
UNUSED(kind);
valkeyClusterAsyncContext *acc = arg;
Expand Down
4 changes: 2 additions & 2 deletions tests/clusterclient_reconnect_async.c
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
/* Unfortunately there is no error code for this error to match */
#define VALKEY_ENOCLUSTER "ERR This instance has cluster support disabled"

void sendNextCommand(int, short, void *);
void sendNextCommand(evutil_socket_t, short, void *);

void connectToValkey(valkeyClusterAsyncContext *acc) {
/* reset context in case of reconnect */
Expand Down Expand Up @@ -61,7 +61,7 @@ void replyCallback(valkeyClusterAsyncContext *acc, void *r, void *privdata) {
event_base_once(acc->adapter, -1, EV_TIMEOUT, sendNextCommand, acc, NULL);
}

void sendNextCommand(int fd, short kind, void *arg) {
void sendNextCommand(evutil_socket_t fd, short kind, void *arg) {
UNUSED(fd);
UNUSED(kind);
valkeyClusterAsyncContext *acc = arg;
Expand Down
4 changes: 2 additions & 2 deletions tests/ct_async_glib.c
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,10 @@ int main(int argc, char **argv) {
valkeyClusterAsyncSetConnectCallback(acc, connectCallback);
valkeyClusterAsyncSetDisconnectCallback(acc, disconnectCallback);

status = valkeyClusterAsyncCommand(acc, setCallback, "id", "SET key value");
status = valkeyClusterAsyncCommand(acc, setCallback, (char*)"id", "SET key value");
ASSERT_MSG(status == VALKEY_OK, acc->errstr);

status = valkeyClusterAsyncCommand(acc, getCallback, "id", "GET key");
status = valkeyClusterAsyncCommand(acc, getCallback, (char*)"id", "GET key");
ASSERT_MSG(status == VALKEY_OK, acc->errstr);

g_main_loop_run(mainloop);
Expand Down
4 changes: 2 additions & 2 deletions tests/ct_connection.c
Original file line number Diff line number Diff line change
Expand Up @@ -306,10 +306,10 @@ void test_command_timeout_set_while_connected(void) {
//------------------------------------------------------------------------------
typedef struct ExpectedResult {
int type;
char *str;
const char *str;
bool disconnect;
bool noreply;
char *errstr;
const char *errstr;
} ExpectedResult;

// Callback for Valkey connects and disconnects
Expand Down
12 changes: 6 additions & 6 deletions tests/ct_out_of_memory_handling.c
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ void test_alloc_failure_handling(void) {
valkeyReply *reply;
const char *cmd = "SET key value";

valkeyClusterNode *node = valkeyClusterGetNodeByKey(cc, "key");
valkeyClusterNode *node = valkeyClusterGetNodeByKey(cc, (char*)"key");
assert(node);

// OOM failing commands
Expand Down Expand Up @@ -265,7 +265,7 @@ void test_alloc_failure_handling(void) {
valkeyReply *reply;
const char *cmd = "SET foo one";

valkeyClusterNode *node = valkeyClusterGetNodeByKey(cc, "foo");
valkeyClusterNode *node = valkeyClusterGetNodeByKey(cc, (char*)"foo");
assert(node);

// OOM failing appends
Expand Down Expand Up @@ -313,8 +313,8 @@ void test_alloc_failure_handling(void) {
prepare_allocation_test(cc, 1000);

/* Get the source information for the migration. */
unsigned int slot = valkeyClusterGetSlotByKey("foo");
valkeyClusterNode *srcNode = valkeyClusterGetNodeByKey(cc, "foo");
unsigned int slot = valkeyClusterGetSlotByKey((char*)"foo");
valkeyClusterNode *srcNode = valkeyClusterGetNodeByKey(cc, (char*)"foo");
int srcPort = srcNode->port;

/* Get a destination node to migrate the slot to. */
Expand Down Expand Up @@ -371,7 +371,7 @@ void test_alloc_failure_handling(void) {
* allowing a high number of allocations. */
prepare_allocation_test(cc, 1000);
/* Fetch the nodes again, in case the slotmap has been reloaded. */
srcNode = valkeyClusterGetNodeByKey(cc, "foo");
srcNode = valkeyClusterGetNodeByKey(cc, (char*)"foo");
dstNode = getNodeByPort(cc, dstPort);
reply = valkeyClusterCommandToNode(
cc, srcNode, "CLUSTER SETSLOT %d NODE %s", slot, replyDstId->str);
Expand Down Expand Up @@ -442,7 +442,7 @@ void test_alloc_failure_handling(void) {

typedef struct ExpectedResult {
int type;
char *str;
const char *str;
bool disconnect;
} ExpectedResult;

Expand Down
2 changes: 1 addition & 1 deletion tests/ct_pipeline.c
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ void test_pipeline(void) {

typedef struct ExpectedResult {
int type;
char *str;
const char *str;
bool disconnect;
} ExpectedResult;

Expand Down
14 changes: 7 additions & 7 deletions tests/ct_specific_nodes.c
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ void test_command_to_all_nodes(valkeyClusterContext *cc) {

void test_transaction(valkeyClusterContext *cc) {

valkeyClusterNode *node = valkeyClusterGetNodeByKey(cc, "foo");
valkeyClusterNode *node = valkeyClusterGetNodeByKey(cc, (char*)"foo");
assert(node);

valkeyReply *reply;
Expand Down Expand Up @@ -73,7 +73,7 @@ void test_streams(valkeyClusterContext *cc) {
char *id;

/* Get the node that handles given stream */
valkeyClusterNode *node = valkeyClusterGetNodeByKey(cc, "mystream");
valkeyClusterNode *node = valkeyClusterGetNodeByKey(cc, (char*)"mystream");
assert(node);

/* Preparation: remove old stream/key */
Expand All @@ -82,7 +82,7 @@ void test_streams(valkeyClusterContext *cc) {
freeReplyObject(reply);

/* Query wrong node */
valkeyClusterNode *wrongNode = valkeyClusterGetNodeByKey(cc, "otherstream");
valkeyClusterNode *wrongNode = valkeyClusterGetNodeByKey(cc, (char*)"otherstream");
assert(node != wrongNode);
reply = valkeyClusterCommandToNode(cc, wrongNode, "XLEN mystream");
CHECK_REPLY_ERROR(cc, reply, "MOVED");
Expand Down Expand Up @@ -235,7 +235,7 @@ void test_pipeline_transaction(valkeyClusterContext *cc) {
int status;
valkeyReply *reply;

valkeyClusterNode *node = valkeyClusterGetNodeByKey(cc, "foo");
valkeyClusterNode *node = valkeyClusterGetNodeByKey(cc, (char*)"foo");
assert(node);

status = valkeyClusterAppendCommandToNode(cc, node, "MULTI");
Expand Down Expand Up @@ -274,11 +274,11 @@ void test_pipeline_transaction(valkeyClusterContext *cc) {
//------------------------------------------------------------------------------
typedef struct ExpectedResult {
int type;
char *str;
const char *str;
size_t elements;
bool disconnect;
bool noreply;
char *errstr;
const char *errstr;
} ExpectedResult;

// Callback for Valkey connects and disconnects
Expand Down Expand Up @@ -487,7 +487,7 @@ void test_async_transaction(void) {
status = valkeyClusterLibeventAttach(acc, base);
assert(status == VALKEY_OK);

valkeyClusterNode *node = valkeyClusterGetNodeByKey(acc->cc, "foo");
valkeyClusterNode *node = valkeyClusterGetNodeByKey(acc->cc, (char*)"foo");
assert(node);

ExpectedResult r1 = {.type = VALKEY_REPLY_STATUS, .str = "OK"};
Expand Down
2 changes: 1 addition & 1 deletion tests/ut_parse_cmd.c
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#include <string.h>

/* Helper for the macro ASSERT_KEY below. */
void check_key(char *key, struct cmd *command, char *file, int line) {
void check_key(const char *key, struct cmd *command, const char *file, int line) {
if (command->result != CMD_PARSE_OK) {
fprintf(stderr, "%s:%d: Command parsing failed: %s\n", file, line,
command->errstr);
Expand Down
Loading