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

Fix small issues 5 #249

Merged
merged 13 commits into from
Jun 21, 2020
5 changes: 5 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
In Development:
# Mentioning Check 0.14.0 for now, to fix distcheck target until next release

* Define CK_ATTRIBUTE_FORMAT for GCC >= 2.95.3, to make use of
‘gnu_printf’ format attribute

* Refactor tests to fix signed - unsigned conversions


Sun Jan 26, 2020: Released Check 0.14.0
based on hash 0076ec62f71d33b5b54530f8471b4c99f50638d7
Expand Down
4 changes: 4 additions & 0 deletions TODO
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,10 @@ Build issues:
[ ] * Add option BUILD_DOCUMENTATION to CMake (Github #217).
[ ] * Missing function check in CMake creates def HAVE_FOO=0.
There should be no HAVE_FOO (Github #195).
[ ] * CMake and Autotools should check if compiler supports __attribute__ ((format (a, b, c)))
to decide if CK_ATTRIBUTE_FORMAT can be defined
https://www.gnu.org/software/autoconf-archive/ax_gcc_func_attribute.html


Check source code:
============
Expand Down
2 changes: 2 additions & 0 deletions lib/libcompat.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)))
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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...

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. Other compilers would get nothing instead of "attribute ((format))" when macro CK_ATTRIBUTE_FORMAT is opened.

#else
#define CK_ATTRIBUTE_UNUSED
#define CK_ATTRIBUTE_FORMAT(a, b, c)
#endif /* GCC 2.95 */

#if GCC_VERSION_AT_LEAST(2,5,0)
Expand Down
4 changes: 2 additions & 2 deletions src/check.c
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ int check_micro_version = CHECK_MICRO_VERSION;

const char* current_test_name = NULL;

static int non_pass(int val);
static int non_pass(enum test_result);
static Fixture *fixture_create(SFun fun, int ischecked);
static void tcase_add_fixture(TCase * tc, SFun setup, SFun teardown,
int ischecked);
Expand Down Expand Up @@ -512,7 +512,7 @@ TestResult **srunner_results(SRunner * sr)
return trarray;
}

static int non_pass(int val)
static int non_pass(enum test_result val)
{
return val != CK_PASS;
}
Expand Down
6 changes: 4 additions & 2 deletions src/check.h.in
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,10 @@ CK_CPPSTART

#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)))
#else
#define CK_ATTRIBUTE_UNUSED
#define CK_ATTRIBUTE_FORMAT(a, b, c)
#endif /* GCC 2.95 */

#if GCC_VERSION_AT_LEAST(2,5,0)
Expand Down Expand Up @@ -499,10 +501,10 @@ static void __testname ## _fn (int _i CK_ATTRIBUTE_UNUSED)
#if @HAVE_FORK@
CK_DLL_EXP void CK_EXPORT _ck_assert_failed(const char *file, int line,
const char *expr,
...) CK_ATTRIBUTE_NORETURN;
...) CK_ATTRIBUTE_NORETURN CK_ATTRIBUTE_FORMAT(gnu_printf, 3, 4);
#else
CK_DLL_EXP void CK_EXPORT _ck_assert_failed(const char *file, int line,
const char *expr, ...);
const char *expr, ...) CK_ATTRIBUTE_FORMAT(gnu_printf, 3, 4);
#endif

/**
Expand Down
2 changes: 1 addition & 1 deletion src/check_error.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ extern jmp_buf error_jmp_buffer;
/* Print error message and die
If fmt ends in colon, include system error information */
void eprintf(const char *fmt, const char *file, int line,
...) CK_ATTRIBUTE_NORETURN;
...) CK_ATTRIBUTE_NORETURN CK_ATTRIBUTE_FORMAT(gnu_printf, 1, 4);
/* malloc or die */
void *emalloc(size_t n);
void *erealloc(void *, size_t n);
Expand Down
4 changes: 3 additions & 1 deletion src/check_str.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
#ifndef CHECK_STR_H
#define CHECK_STR_H

#include "../lib/libcompat.h"

/* Return a string representation of the given TestResult. Return
value has been malloc'd, and must be freed by the caller */
char *tr_str(TestResult * tr);
Expand All @@ -37,6 +39,6 @@ char *tr_short_str(TestResult * tr);
*/
char *sr_stat_str(SRunner * sr);

char *ck_strdup_printf(const char *fmt, ...);
char *ck_strdup_printf(const char *fmt, ...) CK_ATTRIBUTE_FORMAT(gnu_printf, 1, 2);

#endif /* CHECK_STR_H */
2 changes: 1 addition & 1 deletion tests/check_check.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,6 @@ void record_failure_line_num(const int line);
*
* If there are no more lines to read, -1 is returned.
*/
int get_next_failure_line_num(FILE * file);
long get_next_failure_line_num(FILE * file);

#endif /* CHECK_CHECK_H */
23 changes: 10 additions & 13 deletions tests/check_check_master.c
Original file line number Diff line number Diff line change
Expand Up @@ -561,7 +561,7 @@ END_TEST
START_TEST(test_check_failure_lnos)
{
int i;
int line_no;
long line_no;
int passed = 0;
int number_failed;
TestResult *tr;
Expand Down Expand Up @@ -652,10 +652,6 @@ END_TEST
START_TEST(test_check_test_names)
{
int i;
int line_no;
int passed = 0;
int number_failed;
TestResult *tr;

rewind(test_names_file);

Expand Down Expand Up @@ -936,9 +932,9 @@ char* get_next_test_name(FILE * file)

void record_failure_line_num(int linenum)
{
int to_write;
ssize_t written;
int result;
size_t to_write;
size_t written;
int result, chars_printed;
char string[16];

/*
Expand All @@ -947,12 +943,13 @@ void record_failure_line_num(int linenum)
*/
linenum += 1;

to_write = snprintf(string, sizeof(string), "%d\n", linenum);
if(to_write <= 0)
chars_printed = snprintf(string, sizeof(string), "%d\n", linenum);
if(chars_printed <= 0 || (size_t) chars_printed >= sizeof(string))
{
fprintf(stderr, "%s:%d: Error in call to snprintf:", __FILE__, __LINE__);
exit(1);
}
to_write = (size_t) chars_printed;

if(line_num_failures == NULL)
{
Expand All @@ -969,7 +966,7 @@ void record_failure_line_num(int linenum)
written = fwrite(string, 1, to_write, line_num_failures);
if(written != to_write)
{
fprintf(stderr, "%s:%d: Error in call to fwrite, wrote %zd instead of %d:", __FILE__, __LINE__, written, to_write);
fprintf(stderr, "%s:%d: Error in call to fwrite, wrote %zd instead of %zu:", __FILE__, __LINE__, written, to_write);
exit(1);
}

Expand All @@ -981,13 +978,13 @@ void record_failure_line_num(int linenum)
}
}

int get_next_failure_line_num(FILE * file)
long get_next_failure_line_num(FILE * file)
{
char * line = NULL;
char * end = NULL;
size_t length;
ssize_t written;
int value = -1;
long value = -1;

written = getline(&line, &length, file);

Expand Down
8 changes: 4 additions & 4 deletions tests/check_check_sub.c
Original file line number Diff line number Diff line change
Expand Up @@ -1092,7 +1092,7 @@ END_TEST

START_TEST(test_ck_assert_double_eq_with_promotion)
{
float x = 0.1;
float x = 0.1F;
double y = x;

record_test_name(tcase_name());
Expand All @@ -1103,7 +1103,7 @@ END_TEST

START_TEST(test_ck_assert_double_eq_with_conv)
{
float x = 0.1;
float x = 0.1F;

record_test_name(tcase_name());

Expand Down Expand Up @@ -1608,7 +1608,7 @@ END_TEST

START_TEST(test_ck_assert_ldouble_eq_with_promotion)
{
float x = 1.1;
float x = 1.1F;
long double y = x;

record_test_name(tcase_name());
Expand All @@ -1619,7 +1619,7 @@ END_TEST

START_TEST(test_ck_assert_ldouble_eq_with_conv)
{
float x = 1.1;
float x = 1.1F;
long double y = x;

record_test_name(tcase_name());
Expand Down
2 changes: 1 addition & 1 deletion tests/check_check_tags.c
Original file line number Diff line number Diff line change
Expand Up @@ -658,7 +658,7 @@ START_TEST(include_w_spaces)

Suite *make_tag_suite(void)
{
TCase *set_get_tags, *no_filters;
TCase *no_filters;
TCase *include_filters, *exclude_filters;
#if HAVE_DECL_SETENV
TCase *include_filters_env, *exclude_filters_env;
Expand Down
5 changes: 2 additions & 3 deletions tests/check_set_max_msg_size.c
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ static Suite *make_set_max_msg_size_suite(void)

int main (int argc, char *argv[])
{
int n;
SRunner *sr;

if (argc != 2) {
Expand All @@ -64,8 +63,8 @@ int main (int argc, char *argv[])
* Run the test suite. This is intended to trigger the "Message is too long" error.
* Actual success/failure is determined by examining the output.
*/
check_set_max_msg_size(32); /* 1st call has no effect since */
check_set_max_msg_size(atoi(argv[1])); /* the 2nd call will override it. */
check_set_max_msg_size(32); /* 1st call has no effect since */
check_set_max_msg_size(strtoul(argv[1], NULL, 10)); /* the 2nd call will override it. */
sr = srunner_create(make_set_max_msg_size_suite());
srunner_run_all(sr, CK_NORMAL);
srunner_free(sr);
Expand Down