MDEV-39712: gcc 16 error with error=parentheses on mysql/service_encr…#5110
MDEV-39712: gcc 16 error with error=parentheses on mysql/service_encr…#5110grooverdan wants to merge 1 commit into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the ABI check documentation and logic, removes a side-effect assertion from several plugin headers, and integrates Valgrind memory instrumentation into the encryption service. The review feedback identifies portability issues with the use of the __has_include preprocessor operator and GNU-specific sed extensions, and suggests maintaining consistent C-style comments in public headers.
| #if __has_include(<my_valgrind.h>) | ||
| #include <my_valgrind.h> | ||
| #endif |
There was a problem hiding this comment.
The __has_include preprocessor operator is not available in all compilers (it was introduced in GCC 5). Using it directly will cause a compilation error on older compilers that MariaDB still supports. It should be guarded with #ifdef __has_include to ensure portability.
| #if __has_include(<my_valgrind.h>) | |
| #include <my_valgrind.h> | |
| #endif | |
| #ifdef __has_include | |
| #if __has_include(<my_valgrind.h>) | |
| #include <my_valgrind.h> | |
| #endif | |
| #endif |
| EXECUTE_PROCESS( | ||
| COMMAND sed -e "/^# /d" | ||
| -e "/^[ ]*$/d" | ||
| -e "/^[ \t;]*$/d" |
There was a problem hiding this comment.
The use of \t in the sed regex is a GNU extension and is not portable to all sed implementations (e.g., BSD sed on macOS). It is safer to use a literal tab character, as was done in the previous version of this line, to maintain compatibility across different build environments.
-e "/^[ ;]*$/d"
| MEM_CHECK_ADDRESSABLE(dst, *dlen); | ||
| MEM_CHECK_ADDRESSABLE(key, klen); | ||
| MEM_CHECK_ADDRESSABLE(iv, ivlen); | ||
| // Verify buffers do not overlap |
There was a problem hiding this comment.
For consistency with the other comments in this file and to adhere to C-style comment standards for public headers, this should be converted to a block comment /* ... */.
| // Verify buffers do not overlap | |
| /* Verify buffers do not overlap */ |
gkodinov
added
the
MariaDB Foundation
…yption.h The assertion on assignment failed on gcc16. The assertion added in MDEV-30389 seems intent on testing boundaries. Re-use the my_valgrind.h header to approve a bunch of assertions around the contents of the output buffer, and the addressibility of the pointer and lengths passed to the encryption function.
3 participants
…yption.h
The assertion on assignment failed on gcc16.
The assertion added in MDEV-30389 seems intent on testing boundaries.
Re-use the my_valgrind.h header to approve a bunch of assertions around the contents of the output buffer, and the addressibility of the pointer and lengths passed to the encryption function.
note:
very much a test at the moment. Using my_valgrind.h as a header may by appropriate on headers. Having the boundary testing is useful however. The __has_include may not be as portable as desirable.
this is very much a test