Refactor legacy code to bypass warning - Phase 1 #736
+62
−46
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
minminlittleshrimp
force-pushed
the
dlt_devel_fix_cmake_warning
branch
2 times, most recently
from
74e5d02 to
6889860
Compare
added 2 commits
March 15, 2025 12:12
Since CMake 3.20, CMake prefers all source files to have their extensions explicitly listed. File extensions *.c and *.cpp are added to CMakeLists.txt. Update minimum cmake version to 3.10 Signed-off-by: LUU QUANG MINH <[email protected]>
dlt_user: recalculate exact path length buffers dlt_client: correct function type dlt_common: correct function type dlt_multiple_files: recalculate buffer file name dlt_daemon: recalculate exact path length buffers dlt_daemon_common: recalculate exact path length buffers dlt_gateway: correct function type, correct arg type Signed-off-by: LUU QUANG MINH <[email protected]>
minminlittleshrimp
force-pushed
the
dlt_devel_fix_cmake_warning
branch
from
6889860 to
db8a979
Compare
minminlittleshrimp
changed the title
|
Before: After applying this patch: |
|
This PR is splitted from #732 |
|
Hello @lti9hc @Bichdao021195 |
minminlittleshrimp
changed the title
|
Hello @lti9hc |
|
|
||
| if(${CMAKE_VERSION} VERSION_GREATER "3.20" OR ${CMAKE_VERSION} VERSION_EQUAL "3.20") | ||
| cmake_policy(SET CMP0115 OLD) |
There was a problem hiding this comment.
Hi @minminlittleshrimp
Can you give explaination of this code change? Why do we need to remove this checking?
| 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); |
There was a problem hiding this comment.
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);
| 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) |
There was a problem hiding this comment.
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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
cmake: satisfying CMP0115
Since CMake 3.20, CMake prefers all source files to
have their extensions explicitly listed. File extensions
*.c and *.cpp are added to CMakeLists.txt.
Update minimum cmake version to 3.10
lib, shared, daemon, gateway: refactor legacy code
dlt_user: recalculate exact path length buffers
dlt_client: correct function type
dlt_common: correct function type
dlt_multiple_files: recalculate buffer file name
dlt_daemon: recalculate exact path length buffers
dlt_daemon_common: recalculate exact path length buffers
dlt_gateway: correct function type, correct arg type