String handling in C

Does any C or even C++ compiler catch a mismatch like that? That's a terrible bug to have. Code reviews FTW.

gcc looks at the format string for printf & co and gives a warning I think. It has to be built in to the compiler (or via metadata related to a function declaration) since using variable length argument lists removes all checking to do with type and number of arguments...

"String handling in C" is a coding horror all on it's own - no further comment is necessary.

Care to elaborate? :D

I purposely stayed away from C's formatted output for the brief time I was learning C++.

Could be worse, could be a web of pointers so convoluted that they point to nothing while trying to point to some embedded function, with an over-called string in it that still works for some reason. *shudders*

gcc looks at the format string for printf & co and gives a warning I think. It has to be built in to the compiler (or via metadata related to a function declaration) since using variable length argument lists removes all checking to do with type and number of arguments...

Clang also does this, plus, it also checks that the format string is correct with respect to argument types. Really handy!

Care to elaborate?

I purposely stayed away from C's formatted output for the brief time I was learning C++.

You can overflow the buffer at any time, you don't know how long it is, you need to walk over the entire string in order to do any operation (which can easily lead to O(n²) algorithms) - strings in C basically contain everything that one should not do if one was going to design a string library. See http://en.wikipedia.org/wiki/C_string_handling#Criticism and http://www.joelonsoftware.com/articles/fog0000000319.html for more.

Does any C or even C++ compiler catch a mismatch like that? That's a terrible bug to have. Code reviews FTW.

This was an IBM C compiler which didn't perform any such checks. I don't think any did at the time.

The team were supposed to do code reviews and should have picked this up then. My job was developer support which included solving "our code's crashing and we don't know why" type problems. In this case I was given a memory dump and asked to figure out what was going wrong.

Care to elaborate? :D

I purposely stayed away from C's formatted output for the brief time I was learning C++.

Each %s in the string means there should be another parameter that is a pointer to a string. The line should look like:

sprintf(str, "%s %s %s", a, b, c);

Because the function is expecting another parameter on the stack to go with the third %s, it will use whatever is in the next memory location after the b. This could be anything!

Care to elaborate?

I purposely stayed away from C's formatted output for the brief time I was learning C++.

You can overflow the buffer at any time, you don't know how long it is, you need to walk over the entire string in order to do any operation (which can easily lead to O(n²) algorithms) - strings in C basically contain everything that one should not do if one was going to design a string library. See http://en.wikipedia.org/wiki/C_string_handling#Criticism and http://www.joelonsoftware.com/articles/fog0000000319.html for more.

Actually it isn't just strings, it's arrays in general that suffer from that (actually with generic arrays it's even worse - with strings at least you can expect it to stop when there's a zero, with an array the only way to be 100% sure of the length is to pass it separately). Strings just happen to be one specific application of an array (to the point all array operations work on them).

I always wonder when people just use printf-like functions with %s or even without a single %. Dont they know there are things like fputs, strcpy, strcat which dont need to parse possibly wrong format strings?

Then the code above would have been equivalent to this (bug included):

strcpy(str, a);
strcat(str, " ");
strcat(str, b);
strcat(str, " ");

I know that isn't optimal (it'll read all of the string thrice) and you can make it faster, but then the code becomes less clear and can be much harder to read. Not like this code is not error prone anyway - I wonder how many programmers end up reading the strcpy as strcat. So in that sense sprintf looks like a good thing because it makes the code more concise without giving up much on readability (if we're talking about just a single string then it's overkill though).

Care to elaborate?

I purposely stayed away from C's formatted output for the brief time I was learning C++.

C doesn't have strings. It has arrays of characters, and some fancy goggles for the programmer which make those arrays look and act a bit like strings if you're very careful.

Then the code above would have been equivalent to this (bug included):

strcpy(str, a);
strcat(str, " ");
strcat(str, b);
strcat(str, " ");

I know that isn't optimal (it'll read all of the string thrice) and you can make it faster, but then the code becomes less clear and can be much harder to read. Not like this code is not error prone anyway - I wonder how many programmers end up reading the strcpy as strcat. So in that sense sprintf looks like a good thing because it makes the code more concise without giving up much on readability (if we're talking about just a single string then it's overkill though).

char* unknown;
strcpy(str, a);
strcat(str, " ");
strcat(str, b);
strcat(str, " ");
srtcat(str, unknown);

I fixed your bug for you, sir.

"String handling in C" is a coding horror all on it's own - no further comment is necessary.

I'd say "string manipulation in C" is a coding horror, but consuming read-only strings in C is refreshingly lacking in unnecessary abstraction.

In my C++ engine, I don't use any string classes. Instead I choose to use const char* for any strings, simply because I don't do any string manipulation at all, so the simplest solution works fine

[edit] to clarify, this also means not using any of the C standard library functions that work on strings [/edit]

Care to elaborate?

I purposely stayed away from C's formatted output for the brief time I was learning C++.

You can overflow the buffer at any time, you don't know how long it is, you need to walk over the entire string in order to do any operation (which can easily lead to O(n²) algorithms) - strings in C basically contain everything that one should not do if one was going to design a string library. See http://en.wikipedia.org/wiki/C_string_handling#Criticism and http://www.joelonsoftware.com/articles/fog0000000319.html for more.

Care to elaborate?

I purposely stayed away from C's formatted output for the brief time I was learning C++.

Each %s in the string means there should be another parameter that is a pointer to a string. The line should look like:

sprintf(str, "%s %s %s", a, b, c);

Because the function is expecting another parameter on the stack to go with the third %s, it will use whatever is in the next memory location after the b. This could be anything!

Care to elaborate?

I purposely stayed away from C's formatted output for the brief time I was learning C++.

C doesn't have strings. It has arrays of characters, and some fancy goggles for the programmer which make those arrays look and act a bit like strings if you're very careful.

Oh I see then it might trash the memory, thanks!

"String handling in C" is a coding horror all on it's own - no further comment is necessary.

I'd say "string manipulation in C" is a coding horror, but consuming read-only strings in C is refreshingly lacking in unnecessary abstraction.

Only as long as you're reading it sequentially. If you ever need to know the length, you'll need to use strlen which traverses the entire string (and thereby is a performance penalty), and if you're using a variable-length encoding such as UTF-8, consider yourself screwed as all the functions work on chars rather than the proper characters (e.g. in that case strlen would return the number of bytes, rather than the number of characters).

Oh I see then it might trash the memory, thanks!

It's not just that. If the value that happens to be on the stack is invalid if used as an address, it would crash the program.

If you're lucky it trashes the memory and gives you a nice clean crash at the point where things started going wrong.

More normally, it seems to work OK but at some arbitrary point later and in a completely different part of your code you start getting weird things happen.

Yeah, strings are just arrays, but I think it's worth singling out strings here because if you're using an array you've normally got an extra level of awareness of what you're doing, whereas the CRT tries to look like it's pretending that strings are some kind of special case or something different, which may lead the unwary into thinking that they're OK.