3

I'm a total noob in C. I can't make the connect between this function and main. I'm trying to print out a 2d array and I keep getting segmentation fault. Any help would be greatly appreciated.

EDIT: When I changed the last line 'printf("%d:[%s]\n",i,*(p+i))' from %s to %c, I get the first word in the file i'm reading from. So turns out that something is in fact being returned from my function. Now just need to figure out how to get it to return words from other lines in the file. 

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

#define num_strings 20
#define size_strings 20

int *read_file(){
    int j = 0;
    static char text[num_strings][size_strings];

    FILE *fp;
    int x;

    fp = fopen("dictionary2.txt", "r");

    char s[100];
    while(!feof(fp)) {
        x = fscanf(fp,"%[^\n]",s);
        fgetc(fp);

        if (x==1) {
            strcpy(text[j],s);
            j++;
        }
    }
    return text;
}

int main() {
    int *p;
    p = read_file();
    int i;
    for(i = 0; i < 10; i++) {
        printf("%d:[%s]\n",i,*(p+i));
    }
    return(0);
}
Improve this question
8
  • This has got to be the weirdest C code I've ever seen. Commented Oct 26, 2014 at 23:53
  • 2
    ;} looks like an emoticon of some kind Commented Oct 26, 2014 at 23:58
  • You can't return arrays in C, period. Since it's declared static, the best you could do is return a suitable pointer to it. Far better is to declare it in main(), and pass it into your function, and forget about returning it altogether. Check the return from fopen() in case it failed, and don't cast the return from malloc(). Learn to format your code properly, too. Commented Oct 27, 2014 at 0:00
  • I have code in there to check in case fopen() fails. I took it out for purposes of posting only what I am concerned about here, which is accessing this 2d array. Commented Oct 27, 2014 at 0:18
  • char *s=(char*)malloc(100); : memory leak. change to char s[100]; Commented Oct 27, 2014 at 0:18

3 Answers 3

Reset to default
3

In general, you should be creating your array in main() and passing it in, this kind of behavior is very unorthodox. However, if you do insist on doing it this way, you have to return a pointer to your array, since you cannot return arrays in C.

This is the kind of thing you'll need:

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

#define num_strings 20
#define size_strings 20

typedef char (*PARR)[num_strings][size_strings];

PARR read_file(int * wordsread)
{
    static char text[num_strings][size_strings];
    FILE *fp;

    if ( (fp = fopen("dictionary2.txt", "r")) == NULL ) {
        fprintf(stderr, "Couldn't open file for reading\n");
        exit(EXIT_FAILURE);
    }

    char s[100];
    int j = 0;

    while ( j < num_strings && fgets(s, sizeof s, fp) ) {
        const size_t sl = strlen(s);
        if ( s[sl - 1] == '\n' ) {
            s[sl - 1] = 0;
        }

        if ( (strlen(s) + 1) > size_strings ) {
            fprintf(stderr, "String [%s] too long!\n", s);
            exit(EXIT_FAILURE);
        }

        strcpy(text[j++], s);
    }

    fclose(fp);
    *wordsread = j;
    return &text;
}

int main(void)
{
    int wordsread = 0;
    PARR p = read_file(&wordsread);

    for ( int i = 0; i < wordsread; ++i ) {
        printf("%d:[%s]\n", i, (*p)[i]);
    }

    return 0;
}

which, with a suitable input file, outputs:

paul@horus:~/src/sandbox$ ./twoarr
0:[these]
1:[are]
2:[some]
3:[words]
4:[and]
5:[here]
6:[are]
7:[some]
8:[more]
9:[the]
10:[total]
11:[number]
12:[of]
13:[words]
14:[in]
15:[this]
16:[file]
17:[is]
18:[twenty]
19:[s'right]
paul@horus:~/src/sandbox$

Note this only works because you declared your array in read_file() as static - don't return pointers to local variables with automatic storage duration in this way.

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

2 Comments

If I could Vote Up a thousand times here I would THANK YOU!!
I'll settle for a single upvote and an acceptance, if it's what you were looking for and you're inclined to accept the answer.
0

Try moving your #defines back and changing your function header to return a pointer to arrays of size_strings characters, as follows:

    #define num_strings 20
    #define size_strings 20

    char (*read_file())[size_strings] {

Or alternately, with a typedef:

    #define num_strings 20
    #define size_strings 20

    typedef char (*PCharArr)[size_strings];

    PCharArr read_file() {

...and change the type of p in main accordingly:

    char (*p)[size_strings];

That will return (a pointer to the first element of) an array of character arrays, which is more or less equivalent to a 2D array of char.

Improve this answer

Comments

-1

Update, oh I see, you pasted the code from main to the function, I know what happened here, you assumed p[20][20] is the same as a p* or maybe a p**, that's not correct, since now if you do *(p+1), the compiler doesn't know each element in p is 20 wide instead of 1 wide. You approach here should be to declare a pointer to an array of strings in read_file and return that instead:

static char text[num_strings][size_strings];
static char *texts[num_strings]

...
while....
    ....
       if (x==1)
        {strcpy(text[j],s);texts[j]=text[j];j++;}


return texts;

your p should be char* not int*. You also need to terminate the loop if 20 items have been read in.

Improve this answer

2 Comments

Yes i'll get to editing the format later. For now, I just want it to WORK. The txt file it is reading is small and the space I allotted is sufficient. I'm trying to return a pointer to the function. I wrote the code all inside main() and it worked. But my goal here is to be able to call it from a function.
I'll take a look, but code is working now anyway. Thanks for the time and effort, much appreciated.

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.