CODEWITHSUNDEEP
cpointersgetlinestrtok
yasar
yasar

Reputation: 13758

using getline and strtok together in a loop

I am new to C, and trying to implement whoami, as an exercise to myself. I have following code:

#define _POSIX_SOURCE
#include <stdio.h>
#include <sys/types.h>
#include <unistd.h>
#include <string.h> // strtok

int str_to_int(const char *str)
{
    int acc = 0;
    int i;
    for (i = 0; str[i] != '\0'; ++i) {
        acc = (10 * acc) + (str[i] - 48); // 48 -> 0 in ascii
    }
    return acc;
}

int main()
{
    FILE *passwd;
    char *line = NULL;
    size_t line_size;

    passwd = fopen("/etc/passwd","r");

    uid_t uid = getuid();

    while (getline(&line, &line_size,passwd) != -1) {
        char *name = strtok(line,":");
        strtok(line,":"); // passwd
        char *user_id = strtok(line,":");
        if (str_to_int(user_id) == uid) {
            printf("%s\n",name);
            break;
        }
    }

    fclose(passwd);
    return 0;
}

Do I need to save line pointer inside of the while loop. Because I think strtok modifies it somehow, but I am not sure if I need to copy the line, or starting address of the line before I use it with strtok.

Upvotes: 1

Views: 4420

Answers (3)

Simon Woo
Simon Woo

Reputation: 484

I totally agree with geekosaur (and Mark). Paraphrasing his comment, you can modify the above code as following:

while (getline(&line, &line_size, passwd) != -1) {
    char *name = strtok(line,":");
    strtok(NULL,":"); // passwd
    char *user_id = strtok(NULL,":");
    if (str_to_int(user_id) == uid) {
        printf("%s\n",name);
        break;
    }
}

You should pass NULL for the strtok invocations other than the first one.

Upvotes: 0

Mark Wilkins
Mark Wilkins

Reputation: 41242

It might be safer to use strtok_r. It is safer in a multi-threaded situation. That may not apply in this case, but it is sometimes better just to assume that some point any snippet you write might end up in a multi-threaded app. The following is the OP code modified to use strtok_r. 

  char *pos;
  char *name = strtok_r(line,":",&pos);
  strtok_r(NULL,":",&pos); // passwd
  char *user_id = strtok_r(NULL,":",&pos);

And, yes, strtok (and strtok_r) do modify the given input buffer (first parameter). But it can be safe if used properly. Since strtok returns a pointer to a buffer inside the given string, you need to be careful how you use it. In your case, when it breaks out of the loop, name and user_id will point to a value inside the line buffer.

And you maybe should read the man pages for getline. The way you are using it, it returns an allocated buffer that your application is responsible for freeing. That might be what you are aiming for, but I mention it because I don't see a free call for it in the posted code.

Upvotes: 0

geekosaur
geekosaur

Reputation: 61439

strtok is a horrid function. I don't know what documentation you read (if any?) but it both modifies the buffer it is passed and retains an internal pointer into the buffer; you should only pass the buffer the first time you use it on a given line, and pass NULL subsequently so it knows to pick up where it left off instead of starting at the beginning again (which won't actually work quite right because it stomped on the buffer...).

Better, find some other way to parse and stay far away from strtok.

Upvotes: 1

Related Questions