String literal to function and strcat()

[paulm]

I want to create a wrapper for printing statements, to make sure that the EOL-character is set (and only once!), and I want to be able to call this function directly with a string literal or with a char array - like normal print statements.

The check is if the last character matches the specified EOL-char. If not, it is added with strcat. The minimal example below does not work as expected though. When calling the function with a string literal the string is printed twice, i.e. "String literal\nString literal" (see full output below).

I know the behaviour is different between a char array and string literal, because the latter is not automatically null-terminated if sent as argument. That's why I added the null-terminator after strncpy.

If I replace '&EOL' by simply "\n", it works fine, but that of course defies the purpose of having the general EOL.

Can someone explain this behaviour, and how should I fix it?

#include <stdio.h>

const char EOL = '\n';

int main()
{   
    char txt[] = "Hello World";
    msgSend("String literal");
    msgSend(txt);
    return 0;
}

void msgSend(const char *msg) {
	char out[65];

	strncpy(out, msg, strlen(msg));
	out[strlen(msg)] = '\0';
	printf("Output message is %i chars long.\n", strlen(out));
	
	if (out[strlen(out)-1] != EOL) strcat(out, &EOL);
	
	printf("Output message is now %i chars long.\n", strlen(out));
	printf(out);
}

The output:

Output message is 14 chars long.                                                                                                              
Output message is now 29 chars long.                                                                                                          
String literal                                                                                                                                
String literalOutput message is 11 chars long.                                                                                                
Output message is now 26 chars long.                                                                                                          
Hello World                                                                                                                                   
String literal

[salem c]

> strncpy(out, msg, strlen(msg));
You should be using the size of out, not the size of your msg to guide how much to copy

> out[strlen(msg)] = '\0';
So replace \0 with another \0

> if (out[strlen(out)-1] != EOL) strcat(out, &EOL);
You can't point at a character constant and pretend it's a string.

Perhaps this

const char EOL[] = "\n";
...
if (out[strlen(out)-1] != EOL[0]) strcat(out, EOL);

> printf(out);
Don't do this. If someone slips some % characters in there, especially a %n, you're sunk.
Use
printf("%s",out);

[Peter87]

I know the behaviour is different between a char array and string literal, because the latter is not automatically null-terminated if sent as argument.

You mean the former. The latter (i.e. string literals) are guaranteed to be null-terminated.

This is wrong

strncpy(out, msg, strlen(msg));
out[strlen(msg)] = '\0';

This is right (but will chop off characters if msg is longer than 64 characters)

strncpy(out, msg, sizeof(out));
out[sizeof(out) - 1] = '\0';

If I replace '&EOL' by simply "\n", it works fine, but that of course defies the purpose of having the general EOL.

&EOL is not null-terminated. You can either construct an array that you pass to strncat.

const char eol_str[] = {EOL, '\0'};
...
strncat(out, eol_str, sizeof(out)) // not correct (this is tricky)

if (strlen(out) + strlen(eol_str) + 1 < sizeof(out)) strcat(out, eol_str); 
// ^ I think this should work but it might not be the best 
//   way of doing it (I'm not used writing C code) 

Or you can do the concatenation manually.

size_t len = strlen(out);
if (len + 1 < sizeof(out))
{
	out[len] = EOL;
	out[len + 1] = '\0';
}

[paulm]

Thanks for the reply. After some playing around with it, I came to a similar solution.
It's a very good remark you make here:

strncpy(out, msg, sizeof(out));
out[sizeof(out) - 1] = '\0';

And of course the addition of the length check. Still a lot to learn when it comes to this.