Return pointer to array of strings C

Question

I am trying to return an array of strings that are taken from a file. The file looks like this (first line is number of words, and every new line is word). I get some weird output in main part when functions are over. I want to return pointer to array of strings. Note that some part of code that uses printing is for checking my program.

Here is the function that allocates memory:

char *generisiProstor(int n) {
    return (char*)malloc(n*sizeof(char[20])); 
}

This is function for taking words from rijeci.txt and should return pointer to array of strings that contains the words:

char* ucitajRijeci(int n) {

    char  i;
    char *rijeci;

    static const char filename[] = "rijeci.txt";
    FILE *file;
    file = fopen(filename, "r");
    if (file != NULL)
    {
        char line[20];
        int n;
        fscanf(file, "%d", &n);
        rijeci = generisiProstor(n);
        if (rijeci == NULL) { 
            return NULL;
        }
        int i = -1;

        fgets(line, 20, file);        //skipping first line witch is integer and not needed
        while (fgets(line, 20, file) != NULL) 
        {
            printf("%s\n", line);            //normal output
            i++;
            strcpy(rijeci + i, line);
            printf("%s\n", rijeci + i);     //normal expected output
        }
        for (i = 0; i < n; i++) {
            printf("%s\n", rijeci + i);   //wrong output
        }
    }
    return rijeci;
}

Main

int main()
{
    static const char filename[] = "rijeci.txt";
    FILE *file;
    file = fopen(filename, "r");
    char *rijeci;
    int i;
    if (file != NULL)
    {
        char line[20];
        int n;
        fscanf(file, "%d", &n);
        rijeci = ucitajRijeci(n);
        printf("Here is the array: ");
        for (i = 0; i < n; i++) {
            printf("%s ", rijeci+i);  //wrong output
        }
    }
    return 0;
}

You are only returning a single long char array rather than an array of char arrays. — Christian Gibbons, Aug 02 '18 at 19:52
You're overriding the last string, with an offset, returning a garbled string instead of an array of strings — dustinroepsch, Aug 02 '18 at 19:54
Why you open the same file to times? You can open it in main and pass ptr to your file — Nick S, Aug 02 '18 at 19:55

score 2 · Accepted Answer · edited Aug 02 '18 at 20:38

Here you have to use 2-dimensional array (char ** instead of char *). Since you are returning 2-d array you have to declare rijeci as char **rijeci;

Return types of both functions should be also char **.
Change rijeci + i to rijeci[i].
Proper code indentation.

Try this modified code. This will work :-

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

/* generisiProstor */

char **generisiProstor(int n)
{
    char **c; // making 2-d array
    c = (char **)malloc(n * sizeof(char *));
    for (int i = 0; i < n; i++)
    {
        c[i] = (char *)malloc(20 * sizeof(char));
    }
    return c;
}

/*  ucitajRijeci  */

char **ucitajRijeci(int n)
{

    char **rijeci; // change to char **

    static const char filename[] = "rijeci.txt";
    FILE *file;
    file = fopen(filename, "r");
    if (file != NULL)
    {
        char line[20];
        int n;
        fscanf(file, "%d", &n);
        rijeci = generisiProstor(n);
        if (rijeci == NULL)
        {
            return NULL;
        }
        int i = -1;

        fgets(line, 20, file); //skipping first line witch is integer and not needed
        while (fgets(line, 20, file) != NULL)
        {
            printf("%s\n", line); //normal output
            i++;
            strcpy(rijeci[i], line);
            printf("%s\n", rijeci[i]); //changed to rijeci[i]
        }
        for (i = 0; i < n; i++)
        {
            printf("%s\n", rijeci[i]); //changed to rijeci[i]
        }
    }
    return rijeci;
}

/*  main()  */

int main()

{

    static const char filename[] = "rijeci.txt";
    FILE *file;
    file = fopen(filename, "r");
    char **rijeci; // change to char **
    int i;
    if (file != NULL)
    {
        char line[20];
        int n;
        fscanf(file, "%d", &n);
        rijeci = ucitajRijeci(n);
        printf("Here is the array: ");
        for (i = 0; i < n; i++)
        {
            printf("%s ", rijeci[i]); //changed to rijeci[i]
        }
    }
    return 0;
}

clafoutis · Answer 2 · 2018-08-02T20:08:00.920

The first problem you encounter is here:

char *generisiProstor(int n) {

return (char*)malloc(n*sizeof(char[20])); 
}

You want an array of char pointers, but you return a char pointer, or an array of char.

This part should be:

char **generisiProstor(int n) {

return (char**)malloc(n*sizeof(char[20])); 
}

The same problem comes with char *rijeci, you are declaring it as a string or a char pointer.
You should declare it like this char **rijeci (you might want it to be char *(rigeci[20]) in this context) so this will be an array of strings.

If I get your code right another problem might come from this part:

while (fgets(line, 20, file) != NULL) 
    {
        printf("%s\n", line);            //normal output
        i++;
        strcpy(rijeci + i, line);
        printf("%s\n", rijeci + i);     //normal expected output
    }

Earlier in the code, you allocate memory for n words. Here you are reading the line, placing it into line. So when you read the first line i is 0, but you increment it before copying it, so your array has its first occurence unset and you are writing the last word on unallocated memory.

This part should be:

while (fgets(line, 20, file) != NULL) 
    {
        printf("%s\n", line);            //normal output
        strcpy(rijeci + i, line);
        i++
        printf("%s\n", rijeci + i);     //normal expected output
    }

score 1 · Answer 3 · answered Aug 02 '18 at 20:11

If you want to return a pointer to a char array of size 20 you have to declare the function as following:

char (*generisiProstor(int n))[20]
{
    return malloc(n*sizeof(char[20]));
}

The variable which holds the pointer to the arrays is declared as:

char (*rijeci)[20];

rijeci[i] is of type char[20] and you can write your strings there.

score 1 · Answer 4 · answered Aug 02 '18 at 20:31

Do you know the definitions of array and string?

I'll give them to you, as given in the 2011 C-standard:

An array type describes a contiguously allocated nonempty set of objects with a particular member object type, called the element type. […]

A string is a contiguous sequence of characters terminated by and including the ﬁrst null character. […]

Thus, an array is a type derived from a complete object-type, but a string is not a type but a data-structure.

You are very cast-happy. Are you sure forcing the compiler to believe you without cause is a good habit to get into? Also prefer sizeof expr over sizeof (TYPE), as it's harder to get wrong initially or out-of-sync when refactoring later.
Consider reading "Do I cast the result of malloc?".

score 0 · Answer 5 · answered Aug 02 '18 at 20:14

You're allocating the correct amount of memory, but you need to change how you use it. malloc() can only return a "flat" array of characters, so the return value from generisiProstor() is a simple pointer to the first character in the whole array.

The reason it's working initially is that each string overwrites the tail end of the previous string, so when you do the print outs during the read in loop, they show correctly. But even so, the payload of your rijeci array is completely corrupt by the time you've finished reading.

One possible solution is to use a struct to hold your words:

struct Rijec
{
    char rijec[20];
};

and then change generisiProstor(int n) to be this:

struct Rijeci *generisiProstor(int n)
{
    return malloc(n * sizeof(struct Rijec)); 
}

Note that the cast is not needed in C, and indeed should be avoided.

Then, you'll need to change the top of ucitajRijeci() to look like this:

struct Rijec *ucitajRijeci(int n)
{
    struct Rijec *rijeci;
    ...

and in all cases where you're using rijeci + i change that to rijeci[i].rijec.

The net result of this is that when you use i to index a word in the rijeci array, the offset will now be correct.

I know how to do it with structs but i needed to return pointer to array. Thank you for explaining — bakero98, Aug 02 '18 at 20:32
Ahh. In that case you have two choices. Continue as you are, and explicitly multiply all indices accessing the `rijeci` array by 20: e.g. `strcpy(rijeci + i * 20, line);` or use anoopknr's modified solution. Both approaches have their advantages and disadvantages. — dgnuff, Aug 02 '18 at 21:27

Return pointer to array of strings C

5 Answers5