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.

Sign up for GitHub

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

Refactor legacy code to bypass warning - Phase 1 #736

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Refactor legacy code to bypass warning - Phase 1 #736

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5d7e3b4
cmake: satisfying CMP0115
Mar 15, 2025
db8a979
lib, shared, daemon, gateway: refactor legacy code
Mar 15, 2025
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
7 changes: 2 additions & 5 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,8 @@
#######

# Set minimum Cmake version and setup policy behavior
cmake_minimum_required(VERSION 3.3)
cmake_minimum_required(VERSION 3.10)

if(${CMAKE_VERSION} VERSION_GREATER "3.20" OR ${CMAKE_VERSION} VERSION_EQUAL "3.20")
cmake_policy(SET CMP0115 OLD)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @minminlittleshrimp
Can you give explaination of this code change? Why do we need to remove this checking?

endif()
project(automotive-dlt VERSION 2.18.10)


Expand Down Expand Up @@ -190,7 +187,7 @@ if(WITH_DLT_DAEMON_VSOCK_IPC OR WITH_DLT_LIB_VSOCK_IPC)
endif()

if(NOT DLT_USER_IPC_PATH)
set(DLT_USER_IPC_PATH "/tmp")
set(DLT_USER_IPC_PATH "/tmp" CACHE STRING "IPC Path for DLT")
endif()

add_definitions(-DDLT_USER_IPC_PATH="${DLT_USER_IPC_PATH}")
Expand Down
6 changes: 3 additions & 3 deletions include/dlt/dlt_client.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ void dlt_client_register_fetch_next_message_callback(bool (*registerd_callback)(
* @param verbose if set to true verbose information is printed out.
* @return negative value if there was an error
*/
int dlt_client_init_port(DltClient *client, int port, int verbose);
DltReturnValue dlt_client_init_port(DltClient *client, int port, int verbose);

/**
* Initialising dlt client structure
Expand Down Expand Up @@ -196,7 +196,7 @@ DltReturnValue dlt_client_send_log_level(DltClient *client, char *apid, char *ct
* @param client pointer to dlt client structure
* @return negative value if there was an error
*/
int dlt_client_get_log_info(DltClient *client);
DltReturnValue dlt_client_get_log_info(DltClient *client);
/**
* Send an request to get default log level to the dlt daemon
* @param client pointer to dlt client structure
Expand All @@ -208,7 +208,7 @@ DltReturnValue dlt_client_get_default_log_level(DltClient *client);
* @param client pointer to dlt client structure
* @return negative value if there was an error
*/
int dlt_client_get_software_version(DltClient *client);
DltReturnValue dlt_client_get_software_version(DltClient *client);
/**
* Initialise get log info structure
* @return void
Expand Down
2 changes: 1 addition & 1 deletion src/console/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ if(NOT WITH_DLT_CONSOLE_WO_SBTM)
endif()

foreach(target IN LISTS TARGET_LIST)
set(target_SRCS ${target})
set(target_SRCS ${target}.c)
add_executable(${target} ${target_SRCS})
target_link_libraries(${target} dlt dlt_control_common_lib)
set_target_properties(${target} PROPERTIES LINKER_LANGUAGE C)
Expand Down
1 change: 1 addition & 0 deletions src/daemon/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ if (WITH_DLT_UNIT_TESTS)
endif(WITH_SYSTEMD_WATCHDOG OR WITH_SYSTEMD)

add_library(dlt_daemon ${library_SRCS})
target_compile_definitions(dlt_daemon PRIVATE DLT_USER_IPC_PATH="${DLT_USER_IPC_PATH}")
target_link_libraries(dlt_daemon ${RT_LIBRARY} ${SOCKET_LIBRARY} ${CMAKE_THREAD_LIBS_INIT})

install(TARGETS dlt_daemon
Expand Down
4 changes: 2 additions & 2 deletions src/daemon/dlt-daemon.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -363,9 +363,9 @@ int option_handling(DltDaemonLocal *daemon_local, int argc, char *argv[])

#ifdef DLT_DAEMON_USE_FIFO_IPC
snprintf(daemon_local->flags.userPipesDir, DLT_PATH_MAX,
"%s/dltpipes", dltFifoBaseDir);
"%.1014s/dltpipes", dltFifoBaseDir);
snprintf(daemon_local->flags.daemonFifoName, DLT_PATH_MAX,
"%s/dlt", dltFifoBaseDir);
"%.1019s/dlt", dltFifoBaseDir);
#endif

#ifdef DLT_SHM_ENABLE
Expand Down
8 changes: 7 additions & 1 deletion src/daemon/dlt_daemon_common.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -726,10 +726,16 @@ DltDaemonApplication *dlt_daemon_application_add(DltDaemon *daemon,
}
#endif
#ifdef DLT_DAEMON_USE_FIFO_IPC
/* /dltpipes/dlt%d consists of 13 bytes "/dltpipes/dlt" and pid field (INT_MAX = 2147483647 - 32 bit)
* The worst case: "/dltpipes/dlt2147483647"
* Total: 13 + 4 + '\0' = 18 bytes
* DLT_DAEMON_COMMON_TEXTBUFSIZE = 255 bytes
* Space left: 255 - 18 = 237 bytes
*/
if (dlt_user_handle < DLT_FD_MINIMUM) {
snprintf(filename,
DLT_DAEMON_COMMON_TEXTBUFSIZE,
"%s/dltpipes/dlt%d",
"%.237s/dltpipes/dlt%d",
dltFifoBaseDir,
pid);

Expand Down
2 changes: 1 addition & 1 deletion src/examples/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ if(WITH_UDP_CONNECTION)
endif()

foreach(TARGET IN LISTS TARGET_LIST)
set(TARGET_SRCS ${TARGET})
set(TARGET_SRCS ${TARGET}.c)
add_executable(${TARGET} ${TARGET_SRCS})
target_link_libraries(${TARGET} dlt)
set_target_properties(${TARGET} PROPERTIES LINKER_LANGUAGE C)
Expand Down
2 changes: 1 addition & 1 deletion src/gateway/dlt_gateway.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1610,7 +1610,7 @@ int dlt_gateway_forward_control_message(DltGateway *gateway,

int dlt_gateway_process_on_demand_request(DltGateway *gateway,
DltDaemonLocal *daemon_local,
char node_id[DLT_ID_SIZE],
char *node_id,
int connection_status,
int verbose)
{
Expand Down
2 changes: 1 addition & 1 deletion src/gateway/dlt_gateway.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ DltReceiver *dlt_gateway_get_connection_receiver(DltGateway *g, int fd);
* @param verbose verbose flag
* @return 0 on success, -1 otherwise
*/
int dlt_gateway_process_passive_node_messages(DltDaemon *daemon,
DltReturnValue dlt_gateway_process_passive_node_messages(DltDaemon *daemon,
DltDaemonLocal *daemon_local,
DltReceiver *recv,
int verbose);
Expand Down
17 changes: 12 additions & 5 deletions src/lib/dlt_user.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ static DltReturnValue dlt_initialize_socket_connection(void)
}

remote.sun_family = AF_UNIX;
snprintf(dltSockBaseDir, DLT_IPC_PATH_MAX, "%s/dlt", DLT_USER_IPC_PATH);
snprintf(dltSockBaseDir, DLT_IPC_PATH_MAX, "%.1019s/dlt", DLT_USER_IPC_PATH);
strncpy(remote.sun_path, dltSockBaseDir, sizeof(remote.sun_path));

if (strlen(DLT_USER_IPC_PATH) > DLT_IPC_PATH_MAX)
Expand Down Expand Up @@ -401,8 +401,9 @@ static DltReturnValue dlt_initialize_fifo_connection(void)
char filename[DLT_PATH_MAX];
int ret;

snprintf(dlt_user_dir, DLT_PATH_MAX, "%s/dltpipes", dltFifoBaseDir);
snprintf(dlt_daemon_fifo, DLT_PATH_MAX, "%s/dlt", dltFifoBaseDir);
// Explicitly truncate long paths
snprintf(dlt_user_dir, DLT_PATH_MAX, "%.1014s/dltpipes", dltFifoBaseDir);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @minminlittleshrimp
Kindly please consider replacing the magic numbers with #define macros for better readability and maintainability:
Ex:
#define DLT_FIFO_BASE_MAX_LEN 1014
#define DLT_DAEMON_FIFO_MAX_LEN 1019
snprintf(dlt_user_dir, DLT_PATH_MAX, "%.*s/dltpipes", DLT_FIFO_BASE_MAX_LEN, dltFifoBaseDir);
snprintf(dlt_daemon_fifo, DLT_PATH_MAX, "%.*s/dlt", DLT_DAEMON_FIFO_MAX_LEN, dltFifoBaseDir);

snprintf(dlt_daemon_fifo, DLT_PATH_MAX, "%.1019s/dlt", dltFifoBaseDir);
ret = mkdir(dlt_user_dir, S_IRUSR | S_IWUSR | S_IXUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH | S_ISVTX);

if ((ret == -1) && (errno != EEXIST)) {
Expand All @@ -424,7 +425,13 @@ static DltReturnValue dlt_initialize_fifo_connection(void)
}

/* create and open DLT user FIFO */
snprintf(filename, DLT_PATH_MAX, "%s/dlt%d", dlt_user_dir, getpid());
/* /dlt%d consists of 4 bytes "/dlt" and pid field (INT_MAX = 2147483647 - 32 bit)
* The worst case: "/dlt2147483647"
* Total: 4 + 4 + '\0' = 9 bytes
* DLT_PATH_MAX = 1024 bytes
* Space left: 1024 - 9 = 1015 bytes
*/
snprintf(filename, DLT_PATH_MAX, "%.1015s/dlt%d", dlt_user_dir, getpid());

/* Try to delete existing pipe, ignore result of unlink */
unlink(filename);
Expand Down Expand Up @@ -1082,7 +1089,7 @@ DltReturnValue dlt_free(void)
if (dlt_user.dlt_user_handle != DLT_FD_INIT) {
close(dlt_user.dlt_user_handle);
dlt_user.dlt_user_handle = DLT_FD_INIT;
snprintf(filename, DLT_PATH_MAX, "%s/dlt%d", dlt_user_dir, getpid());
snprintf(filename, DLT_PATH_MAX, "%.1015s/dlt%d", dlt_user_dir, getpid());
unlink(filename);
}

Expand Down
4 changes: 2 additions & 2 deletions src/shared/dlt_common.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2304,7 +2304,7 @@ void dlt_buffer_read_block(DltBuffer *buf, int *read, unsigned char *data, unsig
}
}

int dlt_buffer_check_size(DltBuffer *buf, int needed)
DltReturnValue dlt_buffer_check_size(DltBuffer *buf, int needed)
{
if (buf == NULL)
return DLT_RETURN_WRONG_PARAMETER;
Expand Down Expand Up @@ -2456,7 +2456,7 @@ DltReturnValue dlt_buffer_push(DltBuffer *buf, const unsigned char *data, unsign
return dlt_buffer_push3(buf, data, size, 0, 0, 0, 0);
}

int dlt_buffer_push3(DltBuffer *buf,
DltReturnValue dlt_buffer_push3(DltBuffer *buf,
const unsigned char *data1,
unsigned int size1,
const unsigned char *data2,
Expand Down
23 changes: 14 additions & 9 deletions src/shared/dlt_multiple_files.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,15 +120,20 @@ void multiple_files_buffer_file_name(MultipleFilesRingBuffer *files_buffer, cons
char file_index[11]; /* UINT_MAX = 4294967295 -> 10 digits */
snprintf(file_index, sizeof(file_index), "%010u", idx);

/* create log file name */
char* file_name = files_buffer->filename;
memset(file_name, 0, length * sizeof(char));

const size_t size = length - strlen(file_name) - 1;
strncat(file_name, files_buffer->filenameBase, size);
strncat(file_name, MULTIPLE_FILES_FILENAME_INDEX_DELIM, size);
strncat(file_name, file_index, size);
strncat(file_name, files_buffer->filenameExt, size);
files_buffer->filename[0] = '\0';

/* Calculate available space */
size_t remaining = length - 1; // for '\0'
size_t base_length = strnlen(files_buffer->filenameBase, remaining);
size_t ext_length = strnlen(files_buffer->filenameExt, NAME_MAX+1);

snprintf(files_buffer->filename, length, "%.*s%s%s%s%.*s",
(int)base_length, files_buffer->filenameBase, // Base name (truncated if needed)
MULTIPLE_FILES_FILENAME_INDEX_DELIM, // "."
file_index, // 10-digit padded index
"", // Extra safety buffer
(int)ext_length, files_buffer->filenameExt // Extension (truncated if needed)
);
}

unsigned int multiple_files_buffer_get_idx_of_log_file(char *file)
Expand Down
30 changes: 15 additions & 15 deletions src/tests/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,33 +13,33 @@
# For further information see http://www.covesa.org/.
#######

set(TARGET_LIST ${TARGET_LIST} dlt-test-multi-process)
set(TARGET_LIST ${TARGET_LIST} dlt-test-multi-process-client)
set(TARGET_LIST ${TARGET_LIST} dlt-test-user)
set(TARGET_LIST ${TARGET_LIST} dlt-test-client)
set(TARGET_LIST ${TARGET_LIST} dlt-test-stress-user)
set(TARGET_LIST ${TARGET_LIST} dlt-test-stress-client)
set(TARGET_LIST ${TARGET_LIST} dlt-test-stress)
set(TARGET_LIST ${TARGET_LIST} dlt-test-fork-handler)
set(TARGET_LIST ${TARGET_LIST} dlt-test-init-free)
set(TARGET_LIST ${TARGET_LIST} dlt-test-preregister-context)
set(TARGET_LIST ${TARGET_LIST} dlt-test-filetransfer)
set(TARGET_LIST ${TARGET_LIST} dlt-test-multi-process.c)
set(TARGET_LIST ${TARGET_LIST} dlt-test-multi-process-client.c)
set(TARGET_LIST ${TARGET_LIST} dlt-test-user.c)
set(TARGET_LIST ${TARGET_LIST} dlt-test-client.c)
set(TARGET_LIST ${TARGET_LIST} dlt-test-stress-user.c)
set(TARGET_LIST ${TARGET_LIST} dlt-test-stress-client.c)
set(TARGET_LIST ${TARGET_LIST} dlt-test-stress.c)
set(TARGET_LIST ${TARGET_LIST} dlt-test-fork-handler.c)
set(TARGET_LIST ${TARGET_LIST} dlt-test-init-free.c)
set(TARGET_LIST ${TARGET_LIST} dlt-test-preregister-context.c)
set(TARGET_LIST ${TARGET_LIST} dlt-test-filetransfer.c)
install(FILES dlt-test-filetransfer-file dlt-test-filetransfer-image.png
DESTINATION share/dlt-filetransfer)

if(WITH_DLT_CXX11_EXT)
set(TARGET_LIST ${TARGET_LIST} dlt-test-cpp-extension)
set(TARGET_LIST ${TARGET_LIST} dlt-test-cpp-extension.cpp)
endif()

#TODO: Enable again once dlt-test-non-verbose is adapted to non-macro usage
if (NOT WITH_DLT_DISABLE_MACRO)
set(TARGET_LIST ${TARGET_LIST} dlt-test-non-verbose)
set(TARGET_LIST ${TARGET_LIST} dlt-test-non-verbose.c)
endif()

if(WITH_DLT_QNX_SYSTEM)
set(TARGET_LIST ${TARGET_LIST} dlt-test-qnx-slogger)
set(TARGET_LIST ${TARGET_LIST} dlt-test-qnx-slogger.c)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @minminlittleshrimp
"Target names should not include .c or .cpp as they represent logical entities such as executables or libraries, rather than source files. To support both C and C++ files, consider maintaining separate target lists for .c and .cpp files. This approach enhances clarity and ensures proper organization in the build system."
dlt-test-qnx-slogger (✅ Correct) → This should be the target name which is a part of target_list.
dlt-test-qnx-slogger.c (❌ Incorrect) → This is a source file and should be added separately.

endif()

foreach(TARGET IN LISTS TARGET_LIST)
set(TARGET_SRCS ${TARGET})
add_executable(${TARGET} ${TARGET_SRCS})
Expand Down
Loading