Nested strtok in c resulting in an infinite loop

Question

I make user enter a username and I then go to this file and extract the values corresponding the particular user. I know the fault is with the way that I am using strtok as it only works for the first user.

Once I find the user, I want to stop searching in the file.

int fd;
fd=open(fileName,O_RDONLY,0744);

if (fd==-1)
{
    printf("The file userDetails.txt failed to open.\n");
    exit(1);
}

int fileSize = sizeof(fileOutput)/sizeof(fileOutput[0]); //size of file

printf("%d\n",fileSize);

int bytesRead = read(fd,&fileOutput,fileSize);

//TERMINATING BUFFER PROPERLY
fileOutput[bytesRead] = '\0';

printf("%s\n",fileOutput);
//READING LINE BY LINE IN FILE
char *line;
char *data;
char *name;
char *saltValue;
char *encryptedValue;

line = strtok(fileOutput,"\n"); //SPLIT ACCORDING TO LINE

while (line != NULL)
{
    data = strtok(line, ":");
    while (data != NULL)
    {
        name = data;

        if (strcmp(name,userName)==0)
        {
            printf("%s\n","User exists");

            saltValue = strtok(NULL,":");
            printf("%s\n",saltValue);

            encryptedValue = strtok(NULL, ":");
            printf("%s\n",encryptedValue);
            break;
        }
        else
        {
            break;
        }
    }

    if (strcmp(name,userName)==0) //user found
    {
        break;
    }
    else //user not found
    {
        strtok(NULL,"\n");
    }
}

Answer 1

Strtok modifies its input string, which makes impossible to call it in nesting mode, the inner loop workings destroy the work of the outer strtok(), making it impossible to continue.

Appart of this, using strtok() in your problem is not adequate for another reason: if you try to use it to parse the /etc/passwd file (or one of such similar format files that we cope with today) you'll run in trouble with empty fields. In case you have an empty field (two consecutive : chars in sequence, strtok() will skip over both, skipping completely undetected the empty field) Strtok is an old, legacy function that was writen to cope with the three characters (\n\t) that are used to separate arguments in bourne shell. In the case of /etc/passwd you need to cope with possibly empty fields, and that makes it impossible to use strtok() to parse them.

You can easily use strchr() instead to search for the : of /etc/passwd in a non-skipping way, just write something like (you can encapsulate this in a function):

char *not_strtok_but_almost(char *s, char *delim)
{
    static char *p = NULL; /* this makes the function non-reentrant, like strtok() */
    char *saved = NULL;
    if (s) {
        p = s;
        while (strchr(delim, *p)) /* while *p is in the delimiters, skip */ 
            p++; 
        /* *p is not more in the delimiters. save as return value */
        saved = p;
    }
    /* search for delimiters after value */
    while (*p && !strchr(delim, *p)) /* while *p not null, and not in the delimiter set */
        p++;
    /* *p is at the end of the string or p points to one of the delimiters */
    *p = '\0';

    return saved;
}

This function has all the trouble of strtok(3) but you can use it (taking care of its non-reentrancy and that it modifies the source string, making it not nestable on several loops) because it doesn't skip all the separators in one shot, but just stops after the first separator found.

To solve the nesting problem, you can act in a different way, lets assume you have several identifiers separated by spaces (as in /etc/group file) which should require (it doesn't, as the names field is the last, you are not going to call strtok again on the first loop, but to get a NULL. You can process your file in a level first precedence, instead of a depth first precedence. You first seek all fields in the first level, and then go, field by field, reading its subfields (that will use a different delimiter probably)

As all of these modifications are all done in the same string, no need to allocate a buffer for each and strdup() the strings before use... the work can be done in the same file, and strdup()ing the string at the beginning if you need to store the different subfields.

Make any comments if you are in doubt with this (be careful as I have not tested the routine above, it can have probably a bug)

Answer 2

If you are limited to read, that's fine, but you can only use strtok once on "\n" to parse each line from fileOutput, not nested again to parse the ':'. Otherwise, since strtok modifies the string by inserting '\0' at the delimiter found, you will be writing the nul-terminating character within lines that will cause the outer strtok to consider the string finished on the next iteration.

Instead, use a single pointer on each line with strchr (line, ':') to locate the first ':' with the line and then strncmp() using the pointer to the start of line and then pointer locating ':'. For example, if you have a function to check if the userName is contained in your file (returning 0 on success and 1 on failure) you could do:

    ...
    for (char *line = strtok(fileOutput,"\n"); line; line = strtok (NULL, "\n"))
    {
        char *p = strchr (line, ':');                   /* find first ':' */
        if (!p)                                         /* if not found, bail */
            break;
        if (strncmp (userName, line, p - line) == 0) {  /* check name */
            printf ("found user: %s  hash: %s\n", userName, p+1);
            return 0;
        }
    }

    fputs ("user not found.\n", stdout);
    return 1;

This is probably one of the simpler approaches you could take.