// Online C compiler to run C program online
#include <stdio.h>
#include <stdlib.h>

char* word(char w[]) {
    char *temp = w;
    return temp;
}

char** tokens() {
    char *one = word("One");
    char *two = word("two");
    char *three = word("three");
    char *four = word("four");
    char *five = word("five");
    char *six = word("six");
    char *seven = word("seven");
    char *eight = word("eight");
    char *nine = word("nine");
    char *ten = word("ten");
    char *eleven = word("ten");
    char *twelve = word("ten");
    char *thirteen = word("ten");
    
    char *words[13];
    
    words[0] = one;
    words[1] = two;
    words[2] = three;
    words[3] = four;
    words[4] = five;
    words[5] = six;
    words[6] = seven;
    words[7] = eight;
    words[8] = nine;
    words[9] = ten;
    words[10] = eleven;
    words[11] = twelve;
    words[12] = thirteen;
    
    char **pp = words;
    return pp;
}

int main() {
    // Write C code here
    char **pp = (char **)malloc(13 * sizeof(char));
    
    pp = tokens();
    
    for (int i = 0; i < 12; i  ) {
        printf("%s\n", *pp);
        *pp  ;
    }
    // for (; *pp; *pp  ) {
    //     printf("%s\n", *pp);
    // }
    
    return 0;
}

I want the code to print every word contained in the tokens() method. However, in the for loop in main(), it only prints: one, two, three, four, null, and then gets a segmentation fault. If I use the for loop that is commented, it only prints: one, two, three, four. I'm new to C, so pointers are still pretty confusing. I'd appreciate any help. Thanks.

CodePudding user response：

You have quite a few errors:

char **pp = (char **)malloc(13 * sizeof(char));

When allocating memory, you want to allocate sizeof(<oneLevelUp>) * numOfElements. In this case, you're allocating for a char**, so "one level up" is a char*. Additionally, no need to cast the return value of malloc. Change that to

 char **pp = malloc(13 * sizeof(char*));

Finally, the best-considered practice is to include the variable name when using sizeof. This means there will be less maintenance if variable type ever changes:

 char **pp = malloc(13 * sizeof(*pp));  // preferred method

*pp is a char* type. If you change that to

 int **pp = malloc(13 * sizeof(*pp));

now *pp is an int* type automatically, without having to change the sizeof parameter from char* to int*.

The other problems have already been addressed in comments and answers. word essentially does nothing, as it simply returns the argument passed into it; this function will surely get optimized out. @AndreasWenzel's answer explains what's wrong with tokens. The top-rated answer here has an amusing anecdote about returning pointers to local variables.

One more final note, even if tokens didn't invoke UB, you would be/are creating a memory leak by assigning pp to the return value of tokens.

char **pp = malloc(13 * sizeof(*pp));
if (pp == NULL)
{
  // uh oh, we're out of memory, handle the error however you want. For this example, just exit
  exit(-1);
}

// uh oh, by making this assignment, pp no longer points to the memory block
// returned from `malloc`. Now that memory is reserved and nothing points to
// it. If we did this over and over, our process would continue to consume
// memory until none was left.
pp = tokens();

CodePudding user response：

The lifetime of the array words ends as soon as the function tokens returns. Therefore, it is not meaningful for the function tokens to return a pointer to this object, because that pointer will be pointing to an object that no longer exists. Dereferencing that pointer will cause undefined behavior (i.e. your program may crash).

If you want the function tokens to return a pointer to a valid object, you must ensure that the lifetime of words does not end, for example by using dynamic memory allocation (i.e. malloc) or change the lifetime of words to static, by using the static keyword.

In your case, it would be best to remove the function tokens and to simply define the variable words inside the function main, like this:

static const char *const words[] = {
    "one", "two", "three", "four", "five",
    "six", "seven", "eight", "nine", "ten",
    "eleven", "twelve", "thirteen"
};

If you really want to keep the function tokens, then you can rewrite it like this:

const char *const * tokens( void )
{
    static const char *const words[] = {
        "one", "two", "three", "four", "five",
        "six", "seven", "eight", "nine", "ten",
        "eleven", "twelve", "thirteen"
    };

    return words;
}

That way, the function tokens will return a pointer to a statically allocated object, which has static (unlimited) lifetime.