6

I have the following code:

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

int main (void) {
    char str[] = "John|Doe|Melbourne|6270|AU";

    char fname[32], lname[32], city[32], zip[32], country[32];
    char *oldstr = str;

    strcpy(fname, strtok(str, "|"));
    strcpy(lname, strtok(NULL, "|"));
    strcpy(city, strtok(NULL, "|"));
    strcpy(zip, strtok(NULL, "|"));
    strcpy(country, strtok(NULL, "|"));

    printf("Firstname: %s\n", fname);
    printf("Lastname: %s\n", lname);
    printf("City: %s\n", city);
    printf("Zip: %s\n", zip);
    printf("Country: %s\n", country);
    printf("STR: %s\n", str);
    printf("OLDSTR: %s\n", oldstr);

    return 0;
}

Execution output:

$ ./str
Firstname: John
Lastname: Doe
City: Melbourne
Zip: 6270
Country: AU
STR: John
OLDSTR: John

Why can't I keep the old data nor in the str or oldstr, what am I doing wrong and how can I not alter the data or keep it?

pevik
  • 4,523
  • 3
  • 33
  • 44
bsteo
  • 1,738
  • 6
  • 34
  • 60
  • xtmtrx [here in my answer](http://stackoverflow.com/questions/15847194/linked-list-data-all-the-same/15847214#15847214) I written a code that shows how `strtok()` works (it modify string in same address space), I think you should have a look: – Grijesh Chauhan Jun 14 '13 at 09:39
  • 1
    I would expect one to read the source code to the strtok() function or read the function documentation before asking such a question. – PP. Jun 14 '13 at 09:46
  • 1
    So here is [the source code of strkok()](http://www.opensource.apple.com/source/Libc/Libc-166/string.subproj/strtok.c) – Grijesh Chauhan Jun 14 '13 at 09:48
  • Either make a copy of `str` before you call `strtok` or don't use `strtok` and use a pair of pointers to bracket and copy each token, or a combination of `strcspn` and `strspn` to do the same thing. With either of the other methods you can tokenize a string-literal because the original isn't modified, but `strtok` modifies the original by replacing the separator with nul-characters. – David C. Rankin Jul 29 '22 at 02:52

5 Answers5

31

when you do strtok(NULL, "|") strtok() find token and put null on place (replace token with \0) and modify string.

you str, becomes:

char str[] = John0Doe0Melbourne062700AU;
                 
  Str array in memory 
+------------------------------------------------------------------------------------------------+
|'J'|'o'|'h'|'n'|0|'D'|'o'|'e'|0|'M'|'e'|'l'|'b'|'o'|'u'|'r'|'n'|'e'|0|'6'|'2'|'7'|'0'|0|'A'|'U'|0|
+------------------------------------------------------------------------------------------------+
                 ^  replace | with \0  (ASCII value is 0)

Consider the diagram is important because char '0' and 0 are diffident (in string 6270 are char in figure parenthesised by ' where for \0 0 is as number)

when you print str using %s it print chars upto first \0 that is John

To keep your original str unchanged you should fist copy str into some tempstr variable and then use that tempstr string in strtok():

char str[] = "John|Doe|Melbourne|6270|AU";
char* tempstr = calloc(strlen(str)+1, sizeof(char));
strcpy(tempstr, str);

Now use this tempstr string in place of str in your code.

ryyker
  • 22,849
  • 3
  • 43
  • 87
Grijesh Chauhan
  • 57,103
  • 20
  • 141
  • 208
3

Because oldstr is just a pointer, an assignment will not make a new copy of your string.

Copy it before passing str to the strtok:

          char *oldstr=malloc(sizeof(str));
          strcpy(oldstr,str);

Your corrected version:

#include <stdio.h>
#include <string.h>
#include<malloc.h>
int main (void) {

   char str[] = "John|Doe|Melbourne|6270|AU";
   char fname[32], lname[32], city[32], zip[32], country[32];
   char *oldstr = malloc(sizeof(str));
   strcpy(oldstr,str);

    ...................
    free(oldstr);
return 0;
}

EDIT:

As @CodeClown mentioned, in your case, it's better to use strncpy. And instead of fixing the sizes of fname etc before hand, you can have pointers in their place and allocate the memory as is required not more and not less. That way you can avoid writing to the buffer out of bounds......

Another Idea: would be to assign the result of strtok to pointers *fname, *lname, etc.. instead of arrays. It seems the strtok is designed to be used that way after seeing the accepted answer.

Caution:In this way, if you change str further that would be reflected in fname,lname also. Because, they just point to str data but not to new memory blocks. So, use oldstr for other manipulations.

#include <stdio.h>
#include <string.h>
#include<malloc.h>
int main (void) {

    char str[] = "John|Doe|Melbourne|6270|AU";
    char *fname, *lname, *city, *zip, *country;
    char *oldstr = malloc(sizeof(str));
    strcpy(oldstr,str);
    fname=strtok(str,"|");
    lname=strtok(NULL,"|");
    city=strtok(NULL, "|");
    zip=strtok(NULL, "|");
    country=strtok(NULL, "|");

    printf("Firstname: %s\n", fname);
    printf("Lastname: %s\n", lname);
    printf("City: %s\n", city);
    printf("Zip: %s\n", zip);
    printf("Country: %s\n", country);
    printf("STR: %s\n", str);
    printf("OLDSTR: %s\n", oldstr);
    free(oldstr);
return 0;
}
pinkpanther
  • 4,770
  • 2
  • 38
  • 62
  • Good answer! Maybe use `strncpy()` and do not use static buffers as some people or cities have names longer than 32 characters. His next question will be about stack corruption :-) – aggsol Jun 14 '13 at 09:17
  • 1
    In a real program - rather than just something lashed up to ask a question about - don't forget to `free` the memory used by `oldstr` when you've done. – nurdglaw Jun 14 '13 at 09:20
  • @Code Clown, yes, stack corruption :P solved with Grijesh Chauhan answer – bsteo Jun 14 '13 at 09:28
  • Yes, I do `free(str)` and `free(oldstr)` – bsteo Jun 14 '13 at 09:29
  • @pinkpanther here is not, but in the working code my `str` is dynamically allocated (Curl data from web). – bsteo Jun 14 '13 at 09:39
1

strtok requires an writeable input string and it modifies the input string. If you want to keep the input string you have to a make a copy of it first.

For example:

char str[] = "John|Doe|Melbourne|6270|AU";
char oldstr[32];

strcpy(oldstr, str);  // Use strncpy if you don't know
                      // the size of str
ouah
  • 142,963
  • 15
  • 272
  • 331
  • 1
    Better get the length of the string first and create a fitting data as people have different names and live in different cities. – aggsol Jun 14 '13 at 09:13
0

You just copy the pointer to the string, but not the string itself. Use strncpy() to create a copy.

char *oldstr = str; // just copy of the address not the string itself!
aggsol
  • 2,343
  • 1
  • 32
  • 49
0

The for() loop below shows how code calls strtok() at only one location.

int separate( char *flds[], int size, char *fullStr ) {
    int count = 0;
    for( char *cp = fullStr; ( cp = strtok( cp, " " ) ) != NULL; cp = NULL ) {
        flds[ count ] = strdup( cp ); // must be free'd later!
        if( ++count == size )
            break;
    }
    return( count );
}
Fe2O3
  • 6,077
  • 2
  • 4
  • 20