0

I don't get it. I am looking since hours on following function and don't get why I get a memory fault when walking through the array in the end. If I don't walk through it, it runs without error. The function should split the input string first by "|" and then by " ", putting all stuff in a 3 dimensional array. e.g. a string like "ls -l | grep test" should be splitted in every command in first dimension, every argument in second dimension. Third for strings.

I think the error is in dynamically allocating memory. But I don't get it.

char ***splitInput(char *input){

    // Here is splitted data stored (explanation in main)
    char ***commands;

    char *argT=NULL;
    char *commT=NULL;
    char *commTcopy=NULL;
    char *save_commTcopy;
    char *save_input;


    // First filter by | and new line to get commands
    commT = strtok_r(input,"|\n",&save_input);
    int c = 0;
    while(commT != NULL){

        // alloc memory for commands
        //  -> number of commands (c1+1)
        //  -> +1 for NULL command in the end
        commands = realloc(commands,sizeof(*commands)*(c+1+1));
        commTcopy = realloc(commTcopy,sizeof(*commTcopy)*strlen(commT)+1);

        // Make Copy of command
        strcpy(commTcopy,commT);
        printf("Cutting command %s\n",commT);

        // Then filter by space and new line to get arguments of command
        argT=strtok_r(commTcopy," \n",&save_commTcopy);

        char **command;
        // command pointer for easier handling
        command = *(commands+c);
        int c1 = 0;
        while(argT != NULL){
            // alloc memory for arguments
            //  -> number of commands (c1+1)
            //  -> +1 for NULL argument in the end
            command = realloc(command,sizeof(*command)*(c1+1+1));
            // alloc memory for argument string
            *(command+c1) = realloc(*(command+c1),sizeof(**command)*strlen(argT)+1);
            // Copy argument to commands three dimensional array
            strcpy(*(command+c1),argT);
            printf("-- %s\n",argT);
            // next argument
            argT = strtok_r (NULL, " \n",&save_commTcopy);
            c1++;
        }
        *(command+c1)=NULL;
        *(commands+c)=command;

        // get next command
        commT = strtok_r (NULL, "|\n",&save_input);
        c++;
    }
    *(commands+c)=NULL;

    // Walk through array for proof
    /*int c2=0;
    while(*(commands+c2)!=NULL){
        printf("Proof Command %d\n",c2);fflush(stdin);
        char **command;
        command = *(commands+c2);
        int c1=0;
        while(*(command+c1)!=NULL){
            printf("--- %s\n",*(command+c1));fflush(stdin);
            c1++;
        }
        c2++;
    }*/

    return commands;
}
2
  • as you are using c++ I'd recommend you to use a vector or any other stl. you do probably have an error allocating the memory. Commented Apr 26, 2014 at 10:48
  • c2.com/cgi/wiki?ThreeStarProgrammer Commented Apr 26, 2014 at 11:34

1 Answer 1

2

I haven't read thorough your code. But there is at least one issue I've found.

You're using realloc to allocate and resize memory. The realloc function is generally used to resize memory. But if you pass a null pointer to it, it will directly allocate memory instead.

The issue in your code is here:

    //  -> +1 for NULL command in the end
    commands = realloc(commands,sizeof(*commands)*(c+1+1));
    commTcopy = realloc(commTcopy,sizeof(*commTcopy)*strlen(commT)+1);

    //...

    char **command;
    // command pointer for easier handling
    command = *(commands+c);
    int c1 = 0;
    while(argT != NULL){
        // alloc memory for arguments
        //  -> number of commands (c1+1)
        //  -> +1 for NULL argument in the end
        command = realloc(command,sizeof(*command)*(c1+1+1));

At each round of iteration you reallocated commands to a larger array, this is fine. But the issue is the allocated memory cell *(commands + c) contains uninitialized data, which may not be NULL. You saved that value to command and passed it to realloc. realloc just finds the pointer provided is not NULL so it assumes you are reallocating memory, which is in fact not. Thus a crash.

As I've understanded your code, you assigned command to *(commands + c) later so the previous command = *(commands + c) is just redundant, change it to command = NULL may fix your problem.

Sign up to request clarification or add additional context in comments.

5 Comments

+1 You're on to something, but I'm not sure you knew it. The original commands at the top-level of the OP's allocation scheme is NOT initialized to NULLeither, so the initial realloc() uses an indeterminate pointer, and invokes UB. You found it happens as well for each allocation done inside that. I sense a thematic problem =P
@WhozCraig: The top level commands variable has automatic storage (stack) thus is automatically initialized to its default value (NULL).
Whoever told you that doesn't know their arse from a hole in the ground. If it had static linkage it would be zero-initialized. With automatic storage a scalar (which is what a raw pointer is) has no zero-initialization, and is indeterminate. The OP seems to understand this (see the additional initializers for the other pointers immediately below commands). They missed this one.
@WhozCraig: You're right. After some investigating I found that default initialization of non-class-typed variables gets indeterminate values. Maybe it's luck that prevented crash in the first place. Thank you for clarification.
Thank you @XiangyanSun and @WhozCraig this was exacty the problem! I replaced char ***commands; by char ***commands = NULL; and char **command; by char **command = NULL; + erased the unneeded assignment. Now it works.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.