Using strtok in C

[mkab]

Hello everyone. I have a problem using strtok in C. I get a user input from the command line and I want to tokenize it with pipe ("|") as the delimeter and put the result in a double pointer variable. Here's my code:

char** argv;
 char *token;
    token = strtok(userInput, "|");
    while(token != NULL){
        *(argv++) = token;
        token = strtok(NULL, "|");
    }

    *argv = '\0';

I then use this code to verify if it's well tokenized

while(*argv!= NULL)
        {
            if((strcmp(*argv, "|") == 0){
                count = count + 1;
            }
            argv++;
}
printf("%d pipes", count);

But it doesn't work. I can't seem to use strtok. Any ideas please? Thanks

[Narue]

Your use of strtok() is correct. The problem is your use of the uninitialized argv. You need to make sure that all destination strings have been allocated sufficient memory. Compare and contrast:

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

char *copy_str(const char *s)
{
    char *copy = malloc(strlen(s) + 1);
    
    if (copy != NULL)
        strcpy(copy, s);
        
    return copy;
}

char **split_str(const char *s, const char *delim)
{
    /*
        Error handling omitted for brevity
    */
    char **dst = malloc(sizeof *dst);
    char *src = copy_str(s);
    char *tok = strtok(src, delim);
    size_t n = 0;
    
    while (tok != NULL) {
        dst = realloc(dst, ++n);
        dst[n - 1] = copy_str(tok);
        tok = strtok(NULL, delim);
    }
    
    dst[n] = NULL;    
    free(src);
    
    return dst;
}

int main(void)
{
    char **s = split_str("this|is|a|test", "|");
    size_t i;
    
    for (i = 0; s[i] != NULL; i++) {
        printf("'%s'\n", s[i]);
        free(s[i]);
    }
    
    free(s);
    
    return 0;
}

[mkab]

Wow thanks. That's a very nice code. Cleared some confusion I had pointers. I now understand a little bit my problem. There's a little problem with your code. When I get the user input from the command line, I initialise a double pointer variable to split_str

char** arguments;
    int count = 0, i = 0;

    arguments = split_str(userInput, " ");
    while(arguments[i] != NULL){
        if( strcmp(arguments[i], "|") == 0 ){
            count++;
        }
        i++;
    }
printf("%d pipes\n", count);

I get a weird error when executing this. It's almost as if it's executing the commands and the pipe. Here is the error:

*** glibc detected *** ./a: double free or corruption (out): 0x0878e420 ***
======= Backtrace: =========
/lib/i386-linux-gnu/libc.so.6(+0x6b961)[0x40f961]
/lib/i386-linux-gnu/libc.so.6(+0x6d28b)[0x41128b]
/lib/i386-linux-gnu/libc.so.6(cfree+0x6d)[0x41441d]
./a[0x80491dd]
./a[0x8049222]
/lib/i386-linux-gnu/libc.so.6(__libc_start_main+0xe7)[0x3bae37]
./a[0x8048991]
======= Memory map: ========
0011b000-00137000 r-xp 00000000 08:05 261941     /lib/i386-linux-gnu/ld-2.13.so
00137000-00138000 r--p 0001b000 08:05 261941     /lib/i386-linux-gnu/ld-2.13.so
00138000-00139000 rw-p 0001c000 08:05 261941     /lib/i386-linux-gnu/ld-2.13.so
003a4000-004fe000 r-xp 00000000 08:05 261954     /lib/i386-linux-gnu/libc-2.13.so
004fe000-004ff000 ---p 0015a000 08:05 261954     /lib/i386-linux-gnu/libc-2.13.so
004ff000-00501000 r--p 0015a000 08:05 261954     /lib/i386-linux-gnu/libc-2.13.so
00501000-00502000 rw-p 0015c000 08:05 261954     /lib/i386-linux-gnu/libc-2.13.so
00502000-00505000 rw-p 00000000 00:00 0 
00aab000-00aac000 r-xp 00000000 00:00 0          [vdso]
00f01000-00f1b000 r-xp 00000000 08:05 261982     /lib/i386-linux-gnu/libgcc_s.so.1
00f1b000-00f1c000 r--p 00019000 08:05 261982     /lib/i386-linux-gnu/libgcc_s.so.1
00f1c000-00f1d000 rw-p 0001a000 08:05 261982     /lib/i386-linux-gnu/libgcc_s.so.1
08048000-0804a000 r-xp 00000000 08:05 1065463    /home/mkab/Desktop/systeme_exploitation/shell_test/a
0804a000-0804b000 r--p 00001000 08:05 1065463    /home/mkab/Desktop/systeme_exploitation/shell_test/a
0804b000-0804c000 rw-p 00002000 08:05 1065463    /home/mkab/Desktop/systeme_exploitation/shell_test/a
0804c000-0804d000 rw-p 00000000 00:00 0 
0878e000-087af000 rw-p 00000000 00:00 0          [heap]
b7700000-b7721000 rw-p 00000000 00:00 0 
b7721000-b7800000 ---p 00000000 00:00 0 
b78d1000-b78d2000 rw-p 00000000 00:00 0 
b78e4000-b78e8000 rw-p 00000000 00:00 0 
bfeab000-bfecc000 rw-p 00000000 00:00 0          [stack]
Aborted

Do you know why I have this error? Thanks again.

[Narue]

dst = realloc(dst, ++n);

It should be this:

dst = realloc(dst, ++n * sizeof *dst);

And that, my friend, is proof that regardless of your experience or ability, stupid mistakes happen. Especially if you work with higher level language too often and then drop down to a low level language that doesn't hold your hand. ;)

[mkab]

I still have the same error. I don't know...maybe it's the way I get the user input from the command line. This is the code to get the user input

char userInput[1024];
fgets( userInput,sizeof(userInput),stdin )
userInput[strlen(userInput) - 1] = '\0';

Complete code

char userInput[1024];
fgets( userInput,sizeof(userInput),stdin )
userInput[strlen(userInput) - 1] = '\0';
char** arguments;
arguments = split_str(userInput, " ");
while(arguments[i] != NULL){
  if( strcmp(arguments[i], "|") == 0 ){
    count++;
  }
   i++;
}



    //printf("arguments = %s\n", *arguments);
    printf("%d pipes\n", count);

[Narue]

Please post a complete main() function that exhibits the problem. The snippets you posted are clearly not the code you're trying to execute because they won't compile.

[mkab]

char *copy_str(const char *s)
{
char *copy = malloc(strlen(s) + 1);

if (copy != NULL)
strcpy(copy, s);

return copy;
}

char **split_str(const char *s, const char *delim)
{
/*
  Error handling omitted for brevity
  */
char **dst = malloc(sizeof *dst);
char *src = copy_str(s);
char *tok = strtok(src, delim);
size_t n = 0;

while (tok != NULL) {
dst = realloc(dst, ++n * sizeof *dst);
dst[n - 1] = copy_str(tok);
tok = strtok(NULL, delim);
}

dst[n] = NULL;
free(src);

return dst;
}

int main(int argc, char* argv[]){

    int count = 0, i = 0;
    char userInput[1024];

    fgets( userInput,sizeof(userInput),stdin );
    userInput[strlen(userInput) - 1] = '\0';

    char** arguments;
    arguments = split_str(userInput, " ");
    /**check for pipes*/
    while(arguments[i] != NULL){
    if( strcmp(arguments[i], "|") == 0 ){
      count++;
    }
   i++;
}



    //printf("arguments = %s\n", *arguments);
    printf("%d pipes\n", count);
   


    return 0;

}

[Narue]

That code still doesn't compile (which suggests that you're not showing me your actual code), and when I fix the syntax error on your fgets() call, it doesn't fault on my system.

[mkab]

I have a various header files so putting all the code would look really messy. That's why I didn't put everything here. I actually made another c source file independent of all my header files and pasted the code above. It compiles well but when I execute it, it gives me the same error I posted above. Which syntax error did you fix on my fgets()?

EDIT: It's works partially. If I type "a | b". It doesn't work. But if I type "a | b | c". It works. Weird.

[mkab]

I found out that the problem was actually the "free(src)" in split_str function

dst[n] = NULL;
free(src);
 
return dst;

Since the error was about an invalid pointer. I tried commenting that part and everything works. I suspect that it was trying to free a pointer that doesn't exist which kind of doesn't make sense. Not sure if i'm right. I know it's important to free buffers we don't need anymore from the memory but do we need to free(src)? Since it's a local variable, it's reach is just in the function split_str.

[Narue]

I suspect that it was trying to free a pointer that doesn't exist which kind of doesn't make sense.

This is a case where I don't see an immediate problem. It would probably take some debugging, but as I cannot reproduce the error, I can't help too much. It's a shame, because I'd be interested to know exactly what's causing your error.

I know it's important to free buffers we don't need anymore from the memory but do we need to free(src)?

Unless the process lives for a long time, it's not technically necessary. On modern operating systems the memory allocated dynamically will be freed when the process terminates. However, one can't determine conclusively how library functions such as split_str() might be called, so it's an excellent idea to clean up after yourself.

You can get rid of the allocation entirely (and thus the need for free()) by assuming that s is modifiable. This puts the onus on the caller to refrain from calling split_str() with something like a string literal. The reason for that is strtok() modifies the source string, and string literals are read-only:

char **split_str(char *s, const char *delim)
{
    /*
      Error handling omitted for brevity
    */
    char **dst = malloc(sizeof *dst);
    char *tok = strtok(s, delim);
    size_t n = 0;

    while (tok != NULL) {
        dst = realloc(dst, ++n * sizeof *dst);
        dst[n - 1] = copy_str(tok);
        tok = strtok(NULL, delim);
    }

    dst[n] = NULL;

    return dst;
}

[mkab]

Ok no problem. Thanks a lot for your help. I really appreciate it. I learnt a lot from this. Thanks again!