1

I am attempting to split a line read in from a file and copy the data I found into a character array using strcpy() I understand that strcpy() needs a null terminating character at the end of the line otherwise it will seg fault, although I have also tried using a determined buffer size with strncpy() as well as strncat() with the same results. In my snippet I have attempted simply adding a null terminating character at the end of the token.

Here's the snippet of my code that was failing:

    token = strtok(line, s);
    strcpy(update, token);
    token = strtok(NULL, s);
    token = token + '\0';
    strcpy(firstName, token);
    token = strtok(NULL, s);

The first strcpy() call in this function has not failed yet. The second time I call strcpy()it fails every time I try to run it.

The rest of my code can be found here

  • What do you expect to achieve with the statement `token = token + '\0';`? And how are `update` and `firstName` defined? – lurker Apr 09 '16 at 00:20
  • You need to show what `update` is. – John3136 Apr 09 '16 at 00:20
  • 1
    Without knowing how update and first name are defined and/or what they point this question can't be answered. – Pemdas Apr 09 '16 at 00:21
  • I linked the full code because I thought it would be too long to post as text on this page. But update and firstName are character arrays defined inside of a struct. – SuperNoobAttack Apr 09 '16 at 00:48
  • and as for the `token = token + '\0';` statement, I naively hoped that I could just add the null terminating character at the end of `token` so `strcpy()` could stop without reading in a buffer. – SuperNoobAttack Apr 09 '16 at 00:51
  • `char update[1];` --> `char update[2];` or more. – BLUEPIXY Apr 09 '16 at 00:51
  • Possible duplicate of [Definitive List of Common Reasons for Segmentation Faults](http://stackoverflow.com/questions/33047452/definitive-list-of-common-reasons-for-segmentation-faults) – CodeMouse92 Apr 10 '16 at 00:33

2 Answers2

3

you need check null pointer before copy.

token = strtok(line, s);
strcpy(update, token);
token = strtok(NULL, s);
//token = token + '\0';
if(NULL != token)
{
    strcpy(firstName, token);
    token = strtok(NULL, s);
}
thuc do van
  • 131
  • 1
  • 7
  • I didn't notice that the last value that my test print statement was `NULL`. The null terminating character wasn't the issue, I was trying to copy nothing to `firstName` using `strcpy()` which caused the segmentation fault. Thanks! – SuperNoobAttack Apr 09 '16 at 01:17
  • @SuperNoobAttack: If you'd bothered to *read* my answer, you'd have known sooner: "If you second strcpy is failing, it's probably because strtok is failing and returning a null pointer." And I get downvoted for it. "But am I bitter? No. Well, maybe a little." – Jerry Coffin Apr 09 '16 at 04:04
2

strtok returns either a null pointer, or a pointer to a NUL-terminated byte sequence (i.e., a valid C string).

Your token = token + '\0'; isn't doing what you (almost certainly) think. Since token is a pointer (that's the return type from strtok) it's doing pointer arithmetic. Fortunately, '\0' is a long-winded way of saying 0 (trivia of the moment: character literals in C have type int, not type char), so at least it's not changing the pointer.

Check the value being returned by strtok. If you second strcpy is failing, it's probably because strtok is failing and returning a null pointer.

That said, given that you're using a single fixed string to define your delimiters, you can probably do the job a lot easier (and more cleanly) with sscanf. For example, if s contains ,.-+, you could use:

sscanf(input, "%1[^,.-+]%*c%9[^,.-+]", update, firstname);

I've ignored the third call to strtok for now, because you're never copying from its return value to a destination (but repeating the pattern above will work for more variables, of course). Note the use of the number between the % and the scan set ([^...]). You always want to specify the size of buffer when using %s or %[...]. The size you specify should always be one smaller than the buffer into which you're having them write (because they always append a trailing'\0'`).

On the off chance that s is actually a string you read from an external source (or something on that order, so you can't easily put its contents into a string literal) you can synthesize your format string at run-time if needed:

char template[] = "%%[^%s]%%*c%%[^%s]";
char format[512];

sprintf(format, template, s, s);
sscanf(input, format, update, firstname);

[Though format strings are most often literals, that's not required.]

Edit: I don't think you'd originally linked the code, but it reveals at least one problem: char update[1];. Using this as a target of strcpy is a problem because it always appends a trailing '\0' to terminate the string. In this case you've allocated only a single character, so you only have room for the terminator, not any actual data. You need to expand it out to at least two characters. Given that you're copying into fixed-size buffers, you probably want to either check that the string will fit into your buffer before copying, or else use something to copy that limits the size of data it'll copy (and still always includes a terminator, unlike strncpy).

Jerry Coffin
  • 476,176
  • 80
  • 629
  • 1,111
  • All of that and not even a mention that we don;t know what `update` and `firstName` are. My bet is unassigned `char*` – John3136 Apr 09 '16 at 00:38
  • @John3136: It's *possible*, and yes, it's even just barely possible that the first `strcpy` is corrupting memory and leading to the problem with the second. Based on my experience, that isn't *particularly* likely to be the source of the problem though. It certainly could be, but it could just as easily be a fixed buffer that's too short, `s` being uninitialized or empty, so the first call to `strtok` consumes the entire input, etc. – Jerry Coffin Apr 09 '16 at 00:42
  • the `token = strtok(NULL, s);` statement was used to get to the next value of the line I was splitting into tokens. I tested each of these statements by printing them out, and they looked to be the correct values. – SuperNoobAttack Apr 09 '16 at 01:00