Fix small issues 5 #249
Fix small issues 5 #249
Conversation
| @@ -48,8 +48,10 @@ | |||
|
|
|||
| #if GCC_VERSION_AT_LEAST(2,95,3) | |||
| #define CK_ATTRIBUTE_UNUSED __attribute__ ((unused)) | |||
| #define CK_ATTRIBUTE_FORMAT(a, b, c) __attribute__ ((format (a, b, c))) | |||
There was a problem hiding this comment.
What this attribute introduced in GCC 2.95.3? I could not find a reference for that. Do you know when it was introduced?
Or, should there be a configure check for it? Not sure what it would be for CMake, but this may work for autotools:
https://www.gnu.org/software/autoconf-archive/ax_gcc_func_attribute.html
There was a problem hiding this comment.
GNU 2.95.3 is the oldest version "supported" by GNU. I don't know what their official view is but 2.95.3 is the oldest version with documentation on GCC's pages, and attribute ((format)) is supported by 2.95.3. All docs: https://gcc.gnu.org/onlinedocs/
Regarding attribute ((format)): https://gcc.gnu.org/onlinedocs/gcc-2.95.3/gcc_4.html#SEC84
I would not go for a configure check because this is compiler specific thing and can be checked with compiler's versions.
There was a problem hiding this comment.
Actually, thinking about it, it would be good if there was a way to check the compiler's support for attribute ((format)) in the build system, both Autotools and CMake. After all, Clang probably supports all the GNU C originated function attributes.
I think this should go to TODO...
There was a problem hiding this comment.
Really I'm concerned about compilers other than gcc and clang. I'd suspect that the way it is now the other compilers will not reach the format attribute as they would not claim to be GCC.
A TODO is fine
There was a problem hiding this comment.
Yes. Other compilers would get nothing instead of "attribute ((format))" when macro CK_ATTRIBUTE_FORMAT is opened.
|
May you add a note to the NEWS file for these changes, if they should be mentioned in the release notes? Probably the |
mikkoi
force-pushed
the
fix-small-issues-5
branch
2 times, most recently
from
8365a69 to
3341e1a
Compare
function int get_next_failure_line_num(FILE * file) calls long strtol(..) and returns its return value. Signed-off-by: Mikko Koivunalho <[email protected]>
Using GNU C's format attribute: https://gcc.gnu.org/onlinedocs/gcc-2.95.3/gcc_4.html#SEC84 Fixing following warnings: /src/check.c:383:9: warning: function ‘_ck_assert_failed’ might be a candidate for ‘gnu_printf’ format attribute [-Wsuggest-attribute=format] /src/check_error.c:47:5: warning: function ‘eprintf’ might be a candidate for ‘gnu_printf’ format attribute [-Wsuggest-attribute=format] /src/check_str.c:91:9: warning: function ‘ck_strdup_printf’ might be a candidate for ‘gnu_printf’ format attribute [-Wsuggest-attribute=format] Signed-off-by: Mikko Johannes Koivunalho <[email protected]>
man snprintf:
Upon successful return, these functions return the number of characters printed (excluding the null byte used to end output to strings).
The functions snprintf() and vsnprintf() do not write more than size bytes (including the terminating null byte ('\0')). If the output was
truncated due to this limit, then the return value is the number of characters (excluding the terminating null byte) which would have been
written to the final string if enough space had been available. Thus, a return value of size or more means that the output was truncated.
(See also below under NOTES.)
If an output error is encountered, a negative value is returned.
Signed-off-by: Mikko Koivunalho <[email protected]>
Signed-off-by: Mikko Johannes Koivunalho <[email protected]>
Signed-off-by: Mikko Johannes Koivunalho <[email protected]>
mikkoi
force-pushed
the
fix-small-issues-5
branch
from
6b3e6f6 to
28fd45a
Compare
The fail, fail_unless, and fail_if APIS were expected to contain a message explaining the failure. This was never enforced, and it was possible to write unit tests without providing messages. In github.com//pull/249 a change was introduced to add printf argument checking to the Check assert APIS, including the fail APIs. There were a few fixes for this in github.com/libcheck/check/releases/tag/0.15.1. Those changes proved problematic for the uses of the fail* APIs without arguments, as those uses were now flagged as missing the necessary arguments. A fix proposed by heftig in github.com//issues/293 is to add a new NULL to the end of every fail* call in the macro itself. For users of these APIs who do pass a message there will be a new warning about too many arguments. As the fail APIs are deprecated, this new warning is a reasonable trade-off, and can be avoided by switching fail* calls to ck_assert* calls.
Fix some warnings