Problem with concatenation + itoa

Question

I have the following code:

char stringHour[50], stringMinute[50], stringSecond[50];

// lots of code...
itoa(hour, stringHour, 10);
itoa(minute, stringMinute, 10);
itoa(second, stringSecond, 10);

strcat(":", stringSecond);
strcat(":", stringMinute);
strcat(stringMinute, stringSecond);
// stringHour contains both HH and :SS:MM
strcat(stringHour, stringMinute);
drawText(HRES/2 - 4, VRES - GLYPH_HEIGHT*2, 
stringHour, black);

hour, minute and second are all ints.

What I want to do is diplay time as follows: HH:MM:SS.

The variables are guaranteed to be between 0 and 59 (except hout, 0-24).

drawText is correct.

This block of code crashes my program, and I can't find anything wrong with it. Can you?

Thanks for your time!

Answer 1

More than likely the bit that is failing is this piece:

... stuff removed
strcat (":", 
... other stuff removed

You almost certainly can't strcat stuff onto a string literal on the platform you are using.

The strcat approach has buffer overflow safety issues as well. Maybe using snprintf might be a better alternative?

Answer 2

This is wrong:

strcat(":", stringSecond);

You can't use a string literal for the first argument. See strcat doc.

Answer 3

Also, it is best to init arrays to zero.

char stringHour[50] = {0}, stringMinute[50] = {0}, stringSecond[50] = {0};

Then try storing the result of the cat into a string to see what you come up with.

Answer 4

You can't modify string literals; use char buffer[] = "MyLiteral" instead and then strcat() to that.

(The reason behind this fact is that the compiler can optimize code size by consolidating multiple instances of your string literals into one instance in a read-only section of the executable, and so if you could change one instance, you'd change everything else, which would result in unpredictable behavior.)

Answer 5

Also - your strcat is in reverse order. strcat actually "concatenates" - so change the order. strcat(stringHour, ":"); strcat(stringHour, stringMinute); strcat(stringHour, ":"); strcat(stringHour, stringSecond);

However, this is pretty terrible - why not use sprintf? Also - other libraries except for string building - possibly look into the BSL. The C standard library actually has no error checking for you either - so it's a shoot yourself in the foot, unless you know exactly what you're doing and what is expected from the str* family of functions.