Tokenizer not working

Question

I am trying to tokenize a string to give an array of strings but it seems like my code is wrong.

Here is my code:

asmInstruction *tokenizeLine(char *charLine) {
    int words = countTokens(charLine);
    char *tokens = (char*) malloc(MAX_LINE_LENGTH);

    asmInstruction *instr = (asmInstruction*) malloc(sizeof(asmInstruction*));
    instr->args = (char**) malloc(MAX_LINE_LENGTH);

    int count = 1;
    tokens = strtok(charLine, " ,");
    while (count <= words) {
        tokens = strtok(NULL, " ,");
        instr->args[count - 1] = (char*)malloc(MAX_LINE_LENGTH);
        instr->args[count - 1] = tokens;
        ++count;
    }

    free(tokens);
    return instr;
}

 /* Reads a file and returns the number of lines in this file. */
    uint32_t countLines(FILE *file) {
    uint32_t lines = 0;
    int32_t c;
    while (EOF != (c = fgetc(file))) {
        if (c == '\n') {
            ++lines;
        }
    }
    /* Reset the file pointer to the start of the file */
    rewind(file);
    return lines;
}

And the structure:

typedef struct {
    char **args; /* An array of strings*/
} asmInstruction;

My main is here:

int main() {
    char s[] = "ldr r2,r1";
    asmInstruction *instr = tokenizeLine(s);
    printf("%s", instr->args[0]);
}

/* Counts the number of tokens in a line */
uint32_t countTokens(char line[]) {
    /* The correct way to do this! */
    uint32_t numberOfTokens = 0;
    /* Split at spaces and commas */
    char *tokens = strtok(line, " ,");
    while (tokens != NULL) {
        tokens = strtok(NULL, " ,");
        numberOfTokens++;
    }
    return numberOfTokens;
}

So, this should print ldr. However, it prints null. If I loop over the tokens it doesn't print them out but null. I'm expecting to print out the tokens

ldr r2 r1

But only the first one gets printed out.

It seems like instr->args[count-1] never gets assigned something because apparently tokens hasn't been assigned something either.

Why is that? Thanks.

Answer 1

the following code:

handles errors
has many/ most of the logic errors corrected
properly defines the struct asmInstruction
performs the functionality indicated in the question.

suggest elimination of the struct asmInstruction as it is not needed, just use a char** args = NULL; in the tokenizeLine() function and return args.

it is not necessary, nor desirable to malloc memory for 'tokens'. Because that memory pointer will be overlayed each time 'tokens' is set from the returned value from strtok() If a malloc is done, then there will be a memory leak.

In the following code, there still needs to be some additional logic for freeing malloc'd memory and for closing the file before calling 'exit( EXIT_FAILURE );'

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


struct asmInstruction
{
    char **args; /* An array of strings*/
};

#define MAX_LINE_LENGTH (100)

// prototypes
uint32_t countTokens(char line[]);
uint32_t countLines(FILE *file);
struct asmInstruction *tokenizeLine(char *charLine);

int main( void )
{
    char s[] = "ldr r2,r1";
    struct asmInstruction *instr = tokenizeLine(s);
    printf("%s", instr->args[0]);
    return( 0 );
} // end function: main


/* Counts the number of tokens in a line */
uint32_t countTokens(char line[])
{
    /* The correct way to do this! */
    uint32_t numberOfTokens = 0;
    /* Split at spaces and commas */
    char *tokens = strtok(line, " ,");

    while (tokens != NULL)
    {
        tokens = strtok(NULL, " ,");
        numberOfTokens++;
    }
    return numberOfTokens;
} // end function: countTokens


struct asmInstruction *tokenizeLine(char *charLine)
{
    int words = countTokens(charLine);

    char *tokens = NULL;

    struct asmInstruction *instr = NULL;
    if( NULL == (instr = malloc(sizeof( struct asmInstruction)) ) )
    { // then malloc failed
        perror( "malloc for struct asmInstruction failed" );
        exit( EXIT_FAILURE );
    }

    // implied else, malloc successful

    instr->args = NULL;
    if( NULL == (instr->args = malloc(words*sizeof(char*)) ) )
    { // then malloc failed
        perror( "malloc for array of char pointers failed:" );
        exit( EXIT_FAILURE );
    }

    // implied else, malloc successful

    memset( instr->args, '\0', words*sizeof(char*) );

    int count = 0;
    tokens = strtok(charLine, " ,");

    while ( tokens )
    {
        if( NULL == (instr->args[count] = malloc(strlen(tokens)+1) ) )
        { // then, malloc failed
            perror( "malloc for arg failed" );
            exit( EXIT_FAILURE );
        }

        // implied else, malloc successful

        strcpy(instr->args[count], tokens );
        ++count;
        tokens = strtok(NULL, " ,");
    } // end while

    return instr;
} // end function: tokenizeLine


 /* Reads a file and returns the number of lines in this file. */
    uint32_t countLines(FILE *file)
    {
    uint32_t lines = 0;
    int32_t c;

    while (EOF != (c = fgetc(file)))
    {
        if (c == '\n') {
            ++lines;
        }
    }

    /* Reset the file pointer to the start of the file */
    rewind(file);
    return lines;
} // end function: countLines

Answer 2

asmInstruction *tokenizeLine(char *charLine) {
    int words = countTokens(charLine);
    char *tokens;//don't need malloc for this, because just pointer holder.

    asmInstruction *instr = (asmInstruction*) malloc(sizeof(asmInstruction));//allocate size isn't sizeof(asmInstruction*)
    instr->args = (char**) malloc((words+1) * sizeof(char*));//+1 for NULL, or add member E.g instr->numOfWords = words

    int count = 0;
    tokens = strtok(charLine, " ,");
    while (tokens) {
        instr->args[count] = malloc(strlen(tokens)+1);
        strcpy(instr->args[count++], tokens);
        //or  process for each line
        //instr->args[count++] = tokens;//no need allocate for word 
        tokens = strtok(NULL, " ,");//get next tokens
    }
    instr->args[count] = NULL;//set sentinel

    return instr;
}