I was doing update outdated exercises. When I see “what’s changed”, only UNITY_BEGIN and UNITY_END stub was changed, and actual test cases were not changed.
However, after I click “Update Exercise”, my solution couldn’t accepted anymore.
This is error message:
./clock.c: In function 'number_to_clock':
./clock.c:15:22: error: '%02d' directive writing between 2 and 9 bytes into a region of size 6 [-Werror=format-overflow=]
15 | sprintf(r.text, "%02d:%02d", n / 60, n % 60);
| ^~~~
./clock.c:15:21: note: directive argument in the range [-35791370, 23]
15 | sprintf(r.text, "%02d:%02d", n / 60, n % 60);
| ^~~~~~~~~~~
./clock.c:15:21: note: directive argument in the range [-59, 59]
./clock.c:15:5: note: 'sprintf' output between 6 and 14 bytes into a destination of size 6
15 | sprintf(r.text, "%02d:%02d", n / 60, n % 60);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make: *** [makefile:37: tests.out] Error 1
This is related code:
static clock_t number_to_clock(int n) {
clock_t r;
n %= 1440;
if (n < 0) n += 1440;
sprintf(r.text, "%02d:%02d", n / 60, n % 60);
return r;
}
It seems that the compiler is complaining like “Hey, n / 60 and n % 60 can be (technically) negative. If then, your r.text could suffer buffer overflow. I can’t allow such code”
I don’t think this is possible. AFAIK, the result of n %= 1440 is -1439 <= n <= 1439, and then after if (n < 0) n += 1440;, n should be 0 <= n <= 1439.
I think the compiler is trying to be too smart and generated false warning, and then warning treats as error by compiler setting.
Yeah this is a bit weird, as the hours and minutes are always positive and < 60.
What I’ve ended up doing was just to cast them to unsigned int.
Too bad that the exercise tests don’t always fail locally. That made it very confusing to figure out why the code fails on the website.
Did you run Exercism’s test suite locally? Did you enable all the tests by removing or commenting out the TEST_IGNORE(); lines as described in HELP.md?
Yeah, I do all my exerciser locally and the first thing I always do is to remove all TEST_IGNORE() calls. All tests ware green
I have gcc (GCC) 14.2.1 20240805 if that changes anything
Similar to you I can compile the code with gcc-11.4.0 on my computer without warnings but get the same warning (turned error message) when compiling with gcc-12.3.0 or on Exercism’s servers (with gcc-13.2.1).
The answers to this question on StackOverflow claim that this warning gets emitted when the sprintf() or snprintf() might truncate the output or when the returned value is not checked. IMHO that makes sense, I can imagine that to be a common reason for some bugs.
But then I do not understand why we don’t get the warning when the two integers are unsigned.
In my solution I fixed the error with an assert() statement:
int res = snprintf(result.text, sizeof("##:##"),
"%02d:%02d", hours, minutes);
assert(res + 1 == sizeof("##:##"));
tl;dr I can confirm your issue, think the compiler is at least acting inconsistently, and I can only offer a workaround similar to yours.
I’m still learning about inner workings of compilers, C and working with fixed buffer sizes but I think it’s about compiler not being able to figure out actual int variables ranges and thinking that those ints can be negative.
./clock.c: In function 'clock_create':
./clock.c:18:24: error: '%02d' directive writing between 2 and 11 bytes into a region of size 6 [-Werror=format-overflow=]
18 | sprintf(clock.text, "%02d:%02d", hour, minute);
| ^~~~
./clock.c:18:23: note: directive argument in the range [-2147483624, 23]
18 | sprintf(clock.text, "%02d:%02d", hour, minute);
| ^~~~~~~~~~~
./clock.c:18:23: note: directive argument in the range [-2147483588, 59]
./clock.c:18:3: note: 'sprintf' output between 6 and 24 bytes into a destination of size 6
18 | sprintf(clock.text, "%02d:%02d", hour, minute);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make: *** [makefile:37: tests.out] Error 1
Specifically those lines:
./clock.c:18:23: note: directive argument in the range [-2147483624, 23]
and
./clock.c:18:23: note: directive argument in the range [-2147483588, 59]
As I understand casting to unsigned int forces the compiler to limit the range to [0, 23] and [0,59], since unsigned cannot be negative.
I think when it’s just int the compiler is not able to figure out that the numbers will always be positive just from the previous lines.
But if the compiler thinks that minute or hour could be negative, then converting such a negative int to an unsigned int would result in a huge positive number with more than 2 digits and the sprintf() would still write beyond the end of clock.text.
