1

I have a text file and want to save its content into a 2D array, so in which array[i][] I will save a word?

Words are separated with spaces or '\n'

This is the code I made but I always have a segmentation fault. When I copied the data into 1D array everything is going well but when I tried the same thing with the 2D dimension, the code explodes in my face.

Any idea why and would you help me to figure out how to manage my code so it works

I declared the 2D array this way:

char WordsInADictionary [3000][50] = {0};

My function is:

void monterDicoEnMemoire(FILE* fichierDictionary, char **WordsInADictionary) {

    int i = 0, j = 0;
    int caractereIn = 0;

    caractereIn = fgetc(fichierDictionary); 

    while (caractereIn != EOF) { 
        if (caractereIn != '\n' || caractereIn != ' ') {
            WordsInADictionary [i][j] = caractereIn;
            ++j;
        } else { 
            j = 0;
            ++i;
        }

        caractereIn = fgetc(fichierDictionary);
    }
} 
Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
H_B
  • 37
  • 6
  • Post 5-10 lines of input. I presume you just have 1-dictionary word per-line, but it would help to be sure. In that case, read each line with `fgets` and then trim the trailing `'\n'`. Or if there are multiple words per-line separate by whitespace, just loop `while (fscanf (fichierDictionary, %49s, WordsInADictionary[i]) == 1) i++;` – David C. Rankin Jul 27 '19 at 04:30
  • not always, I do have a dictionary where words are separated only by spaces – H_B Jul 27 '19 at 04:34
  • If the words cannot contains spaces, then you can use `fscanf` to read words separated by `'\n'` or `' '` (a newline is considered whitespace). In your code above, you need to add `else { WordsInADictionary [i][j] = 0; ...` to *nul-terminate* each word. Then your code should work. – David C. Rankin Jul 27 '19 at 04:35

2 Answers2

2

Few problems I see here are,

You need to use && not || in your if condition.

            if (caractereIn != '\n' || caractereIn != ' ') {

should be

        if (caractereIn != '\n' && caractereIn != ' ') {

And, you should null terminate the words as below.

      if (caractereIn != '\n' && caractereIn != ' ') {
           WordsInADictionary [i][j] = caractereIn;
           ++j;
      } else { 
           WordsInADictionary[i][j] = '\0';
           j = 0;
           ++i;
      }
kiran Biradar
  • 12,700
  • 3
  • 19
  • 44
2

You have two problems. char** WordsInADictionary is incompatible with char (*WordsInADictionary)[50]. They are different types. Your parameter is a pointer-to-pointer to char. If you dereference it, you get a simple pointer.

On the other hand, your 2D array when passed, decays to a pointer-to-array of char[50]. (a 2D array is just a 1D array of arrays) See: C11 Standard - 6.3.2.1 Other Operands - Lvalues, arrays, and function designators(p3). If you dereference it, you get a array of char [50].

Since the type controls pointer arithmetic, each j is moving by 1-byte, but when you increment i you move by 8-bytes instead of 50-bytes required to reach the beginning of the next 1D array.

To fix the problem,

void monterDicoEnMemoire (FILE* fichierDictionary, char (*WordsInADictionary)[50] ) {

If you will use each array[i] as a string, add:

            } else { 
                WordsInADictionary [i][j] = 0;   /* nul-terminate word */
                j = 0;
                ++i;
            }

And... you will need to fix your conditional expression, See the answer of @kiranBiradar where he explains the issue there.

If the words cannot contain spaces, a simple way to read them into the array would be:

#define ROWS 3000
#define COLS 50
...
size_t monterDicoEnMemoire (FILE* fichierDictionary, 
                            char (*WordsInADictionary)[COLS])
{
    size_t i = 0;

    while (i < ROWS && 
            fscanf (fichierDictionary, "%49s", WordsInADictionary[i]) == 1)
        i++;

    return i;    /* return the number of words in array */
}

(always choose a meaningful return)


Edit Following Comment of Continued Problems

Given your comment:

I've tried the code with fscanf cause it's really easy to write ans simple to understand. but when I try with a printf to check the data. I found that first thing it works only for the first word, with

printf("dico actuel: %s\n", WordsInADictionary[ 0 ]); 

if I tried a printf on WordsInADictionary[ 1 ] and so on .. it's always an empty word, second point, this warning: format ‘%s’ expects argument of type ‘char *’, but argument 3 has type ‘char **’ [-Wformat=] while (i < ROWS && fscanf(fichierDictionary, "%49s", WordsInADictionary [ i ] ) == 1)

You have something you have not implemented correctly. The printf you show is fine and should work (so long as you added it to the monterDicoEnMemoire function. The warning: is not possible if you have made the changes to provide:

size_t monterDicoEnMemoire (FILE* fichierDictionary, 
                            char (*WordsInADictionary)[COLS]) {
    ...
    while (i < ROWS &&  /* protect array bounds while reading into array */
            fscanf (fichierDictionary, "%49s", WordsInADictionary[i]) == 1)
        i++;

To help you step through the problem I wrote a quick example using what is posted above:

#include <stdio.h>

#define ROWS 3000   /* if you need a constant, #define one (or more) */
#define COLS 50

size_t monterDicoEnMemoire (FILE* fichierDictionary, 
                            char (*WordsInADictionary)[COLS])
{
    size_t i = 0;       /* word counter */

    while (i < ROWS &&  /* protect array bounds while reading into array */
            fscanf (fichierDictionary, "%49s", WordsInADictionary[i]) == 1)
        i++;

    return i;   /* return the number of words read */
}

int main (int argc, char **argv) {

    char array[ROWS][COLS] = { "" };    /* 2D array to hold words */
    size_t n = 0;                       /* number of words read */
    /* use filename provided as 1st argument (stdin by default) */
    FILE *fp = argc > 1 ? fopen (argv[1], "r") : stdin;

    if (!fp) {  /* validate file open for reading */
        perror ("file open failed");
        return 1;
    }

    if ((n = monterDicoEnMemoire (fp, array)))  /* read into array */
        for (size_t i = 0; i < n; i++)          /* output each word */
            printf ("array[%2zu] : %s\n", i, array[i]);

    if (fp != stdin)    /* close file if not stdin */
        fclose (fp);
}

Example Input File

The following is a 20-word dictionary file, with one-word, and multiple-words per-line:

$ cat dat/dictwrds20.txt
a
AAA
AAAS
aardvark Aarhus Aaron ABA
Ababa
aback
abacus abalone abandon abase
abash
abate abbas abbe
abbey
abbot
Abbott

Example Use/Output

When you run the program providing the filename to read as the first argument to your program (or reading from stdin by default if no argument is given), you will get:

$./bin/readdict dat/dictwrds20.txt
array[ 0] : a
array[ 1] : AAA
array[ 2] : AAAS
array[ 3] : aardvark
array[ 4] : Aarhus
array[ 5] : Aaron
array[ 6] : ABA
array[ 7] : Ababa
array[ 8] : aback
array[ 9] : abacus
array[10] : abalone
array[11] : abandon
array[12] : abase
array[13] : abash
array[14] : abate
array[15] : abbas
array[16] : abbe
array[17] : abbey
array[18] : abbot
array[19] : Abbott

Look through what you have and see if you can identify where your code varies from the example above.

My Guess At Why Your printf Failed

Given your description in the comments, when you added the printf, I suspect your forgot to add enclosing braces ({ ... }) to create a multi-line block below the while loop. If my guess is correct, you would have needed to do:

    while (i < ROWS &&  /* protect array bounds while reading into array */
            fscanf (fichierDictionary, "%49s", WordsInADictionary[i]) == 1) {
        printf ("%s\n", WordsInADictionary[i]);
        i++;
    }

Since the while loop iterates over more than a single-line, you must enclose all lines within the loop in {...} -- as above. (which I simply omitted when the only statement within the while loop was i++;)

David C. Rankin
  • 81,885
  • 6
  • 58
  • 85
  • 2
    That just shows 2-pair of eyes is always better than one. I blew right by the conditional you caught. – David C. Rankin Jul 27 '19 at 05:01
  • I've tried the code with fscanf cause it's really easy to write ans simple to understand. but when I try with a printf to check the data. I found that first thing it works only for the first word, with printf("dico actuel: %s\n", WordsInADictionary[ 0 ]); if I tried a printf on WordsInADictionary[ 1 ] and so on .. it's always an empty word, second point, this warning: format ‘%s’ expects argument of type ‘char *’, but argument 3 has type ‘char **’ [-Wformat=] while (i < ROWS && fscanf(fichierDictionary, "%49s", WordsInADictionary [ i ] ) == 1) – H_B Jul 27 '19 at 06:40
  • Huh? `argument 3 has type ‘char **’ [-Wformat=] while (i < ROWS && fscanf(fichierDictionary, "%49s", WordsInADictionary[ i ]` that doesn't make any sense. Can you edit your question and add your current code so I can see how you have everything declared? – David C. Rankin Jul 27 '19 at 06:51
  • this was the output after compilation. Tell me please, I'm new with pointers, how can I iterate with the code fscanf to see all the words please – H_B Jul 27 '19 at 07:08
  • See the example I just provided. Ah -- I bet you forgot to add `{ ... }` for the `while` loop when you added `printf` that caused `i` not to increment `:)` (that's just my educated guess) I'll add a short example showing that too. – David C. Rankin Jul 27 '19 at 07:08
  • @DavidC.Rankin it's very clear, thank you very very much for your help. Obviously I have to improve my knowledge with pointers. Do you have a particular book that you can advise me to read for C – H_B Jul 27 '19 at 23:56
  • A pointer is simply a normal variable that holds the *address of* something else as its value. In other words, a pointer *points to* the memory address where something else can be found. For example, while `int a = 5;` stores the immediate value `5` as its value, `int *b;` creates a pointer to `int`, and `b = &a;` stores the address of `a` as `b`'s value (the memory address where `5` is stored). To access the value pointed to by a pointer, you *dereference* the pointer using the unary `'*'` operator, e.g. `int c = *b;` will initialize `c = 5`). Assigning `*b = 6;` makes `a` 6, but `c` stays 5 – David C. Rankin Jul 28 '19 at 00:49
  • Pointer arithmetic is dictated by the *type*. With `char *p`, incrementing `p++` causes the pointer to increment 1-byte (a `char` is 1-byte). With `int *p` incrementing `p++` causes the pointer to increment by 4-bytes (e.g. `sizeof(int)` bytes) so it now points to the next `int`. You prefer passing pointers as parameters because the `sizeof(the_pointer)` is only 8-bytes (4-bytes on x86) compared to passing say the `struct` itself which could require copying of thousands of bytes. There are many good tutorials, but this is the nuts-and-bolts of pointers. – David C. Rankin Jul 28 '19 at 00:55
  • [C - Pointing to Data (not obsolete)](http://www.tutorialspoint.com/ansi_c/c_pointing_data.htm), and [C - Pointers (new "light" ver.)](https://www.tutorialspoint.com/cprogramming/c_pointers) and the [How do pointer to pointers work in C? (many aspects)](https://stackoverflow.com/questions/897366/how-do-pointer-to-pointers-work-in-c) and lastly [C Function Pointer](https://www.zentut.com/c-tutorial/c-function-pointer/) – David C. Rankin Jul 28 '19 at 01:01
  • One last question, what does mean "%49s" in the fprinf instruction ? – H_B Jul 28 '19 at 01:02
  • `49` is a *field-width* modifier to the `%s` *conversion specifier*. It tells `fscanf` to read no more than `49` characters (which is a good thing since you have `array[3000][50]`, e.g. your strings are limited to `50` chars) You limit to `49` so you preserve room for the `+1` needed for the *nul-terminating* character at the end. It is another way to ***protect your array bounds*** `:)` With `fgets` you do NOT need to subtract 1, so `fgets (array[i], 50, fp)` is fine, `fgets` includes the *nul-terminating* character within that limit -- `fscanf` does not. – David C. Rankin Jul 28 '19 at 01:05
  • Finally, learning C is not a race, it is more of a journey. Learning C is like eating a whale, you have to take it "one byte at a time". C is an exact language. You are responsible for every byte of memory. There are no training-wheels. (that is what makes C so blazingly fast) That is also what puts total responsibility on the programmer to use memory correctly. C will happily let you try and read 100-bytes into an array of only 50-bytes. Always compile with warnings enabled (`-Wall -Wextra -pedantic -Wshadow` for gcc/clang, or `/W3` for VS) don't accepts code until there are zero warnings. – David C. Rankin Jul 28 '19 at 01:17