2

I am trying to split the string and print the tokens. 

int main()
{
    char line[255] = "182930101223, KLA1513";
    char val1[16];
    char val2[7];

    strcpy(val1, strtok(line, ","));
    strcpy(val2, strtok(NULL, ","));

    printf("%s|%s\n", val1, val2);

    return 0;
}

When I print I get 

  3| KLA1513

instead of

182930101223| KLA1513

What is the issue?

meaning-matters
  • 21,929
  • 10
  • 82
  • 142
  • 2
    unrelated, but `val2` is too small for that string. You are hitting undefined behaviour there and getting lucky (or rather, unlucky because it didn't shout at you for it) – Dave Aug 24 '13 at 15:23
  • Also, I cannot reproduce the problem: http://codepad.org/Se7gwr5O – Dave Aug 24 '13 at 15:25
  • working fine ....! OUTPUT: **182930101223| KLA1513** – Gangadhar Aug 24 '13 at 15:27
  • 1
    FWIW (and to my considerable surprise, I might add) the program crashed for me with GCC 4.8.1 on Mac OS X 10.8.4 without printing anything. There was an 'Abort trap: 6' signal killing it. When I changed the size of `val2` to 16 (from 7), it produced the output I expected. I was planning to see whether the output differed when the order of declaration for `val1` and `val2` changed; it didn't — it crashed both ways. So, the undefined behaviour identified by Dave is indeed critical (on some platforms, with some compilers). – Jonathan Leffler Aug 24 '13 at 15:55
  • Follow-up: the crash was a runtime check by the compiler/library; the function `__strcpy_chk()` was called and that generated the trap deliberately by calling `__chk_fail()` — a state of affairs that might be regarded as nannyism by the compiler that wasn't really called for. OTOH, it was pretty hard to ignore the fact that `val2` is too small. – Jonathan Leffler Aug 24 '13 at 16:06

2 Answers2

7

OK, I thought this was an unrelated issue, but I now think it is the issue you are seeing.

It's important to remember that C strings take 1 extra character than they contain. This is the terminating NUL character (\0), and marks the end of the text.

So KLA1513 is actually 8 characters (KLA1513\0). Additionally, because you are not trimming spaces, it is 9 characters! _KLA1513\0 (_ is a space).

That means that you're overrunning the memory in your second strcpy, leading to undefined behaviour, which (you will come to realise) is your worst nightmare.

When you print it, who knows what state the program is in. Maybe the memory you overwrote was part of the print call, or maybe it was overwritten again and now var2 isn't terminated.

Just make var2 bigger (9 characters is enough here), and in future, use the safe forms (strncpy, for example). Mistakes like this are how hackers usually manage to compromise systems.

Dave
  • 44,275
  • 12
  • 65
  • 105
2

Try this:

#include <stdio.h>
#include <string.h>

int main()
{
    char line[] = "182930101223, KLA1513";
    char* val1;
    char* val2;

    val1 = strtok(line, ",");
    val2 = strtok(NULL, ",");

    printf("%s|%s\n", val1, val2);

    return 0;
}

There is no need to strcpy() the tokens, you can just use char*; strtok() will add a closing \0 at the end of each token found.

It's also safer, because you don't need to know the size of the tokens beforehand; and size of your tokens was the problem. If you do need to have the token in their own memory, you can copy them into sufficiently sized strings afterwards.

Note that we can't do:

char* line = "182930101223, KLA1513";

because strtok() modifies the string, and it's not allowed to modify a literal C string.

meaning-matters
  • 21,929
  • 10
  • 82
  • 142