0

I need to store some data from a text file into variables. These variable gives me the size of Matrix. Now I need to read matrix from file and store it then use double pointer of struct to point the matrix. Here's my code so far:

Structure that I am using -

struct data{
        char str[20];
        int row, col;
        int **arr;
};

Needed information extracted into variables -

f = fopen("input.txt","r");
        fscanf(f,"%d",&num);
        printf("%d\n",num);
        fgets(NULL,NULL,f);
        fscanf(f,"%s %d %d",ptr->str,&(ptr->row),&(ptr->col));

Now using the row and column values I need to read a matrix from the same file store this information into double pointer of struct.

Something like this:

m = (int **)malloc((ptr->row)*(ptr->col)*(sizeof(int)));

 for(i=0;i<ptr->row;i++){
                for(j=0;j<ptr->col;j++){
                        fscanf(f,"%d",&m[i][j]);
                }
        }

But this last part throws Seg Fault. Please help - how should I go about?

For Reference I am sharing the matrix as well:

50                                                                               
BOROVETS 58 24                                                                                                                    
1090 1091 1060 1137 542 52 768 721 620 737 108 660 1069 789 356 1361 1255 1014 1171 494 57 565 122 1252
1052 1118 17 490 594 22 1375 1070 571 236 82 87 1170 266 873 1340 529 1164 129 700 255 1041 972 927
388 1324 420 1077 8 237 991 181 782 531 94 248 676 1322 498 480 867 816 743 932 701 167 551 569
192 992 588 933 381 467 667 900 1072 670 509 1027 485 644 62 1264 432 400 26 357 453 1260 998 1244
530 805 1360 843 37 1257 144 196 137 749 1154 575 96 206 164 466 1381 703 746 112 549 760 148 621
1138 1219 1005 977 308 1057 303 367 518 326 333 817 354 31 353 1022 136 421 305 1348 652 208 846 320
395 1291 1332 1315 653 872 1097 1007 340 1231 517 745 1008 732 631 1355 501 835 1100 445 514 987 636 578
619 882 880 439 327 437 1127 361 662 1243 163 965 275 1076 832 55 207 787 952 1073 76 109 335 1028
964 103 185 80 435 1305 694 339 1034 413 239 1001 626 1321 169 1325 1282 97 1262 906 212 974 554 1123
1247 566 1237 41 214 996 287 1298 240 256 1012 1035 829 1049 960 1223 416 1018 64 45 706 90 959 175
674 1283 491 1365 277 753 1277 1232 43 546 1011 771 1006 282 792 601 813 288 683 707 162 2 651 673
699 46 604 779 291 436 125 1009 1272 1085 300 1238 343 482 1165 844 671 623 661 1301 1048 550 213 155
610 486 794 797 727 1167 319 950 1046 1186 985 1044 1285 1230 202 831 858 449 143 856 1194 49 724 754
1126 177 119 830 35 1183 456 193 1039 1199 695 450 687 731 114 635 1019 864 1320 800 158 1284 630 1056
1088 562 88 625 1188 161 313 845 176 1016 198 1225 1302 924 634 532 165 1051 417 711 696 1334 465 847
886 639 1117 1066 643 130 51 819 1376 1113 166 265 390 903 702 999 228 492 284 411 424 712 572 1161
1236 75 369 1354 329 124 545 1368 274 1198 231 1300 234 896 306 138 1248 561 951 919 814 519 1271 573
222 1202 146 1349 159 512 127 377 1326 934 1101 358 1136 123 295 668 1344 1130 570 430 48 1196 199 261
1058 92 940 324 243 1265 663 547 344 877 302 1295 1218 778 1313 1319 1156 586 962 259 387 201 1251 785
656 540 593 1205 487 1387 504 328 1182 497 444 680 1105 576 496 60 1329 1120 790 865 69 1312 227 1353
755 1352 405 460 911 516 1042 1178 897 1204 714 29 1241 1083 475 1184 341 331 684 378 1346 1333 351 622
1169 596 120 1020 271 1061 837 270 1158 763 1038 581 920 1110 468 1 318 1155 675 534 178 657 1121 693
642 1145 1108 471 376 230 401 654 1122 502 564 995 1369 851 1089 218 928 579 267 1372 577 898 1309 440
310 1364 14 741 557 438 1279 479 483 781 1166 422 993 836 734 50 904 1287 1356 463 1374 511 807 472
1115 759 101 116 777 406 1017 527 398 1197 801 567 1337 451 747 382 1098 285 1036 1033 476 1246 713 407
393 1125 1114 1040 946 89 1071 98 1210 975 191 370 402 535 296 6 104 442 1245 197 1224 186 769 853
1086 1132 1268 583 355 312 1193 915 322 976 332 447 1030 947 78 891 828 86 190 350 820 25 30 607
875 690 56 935 1131 263 936 825 1010 1384 366 172 131 1208 1207 1037 892 412 1308 1149 742 1373 1151 189
861 1103 105 1358 883 258 1133 379 614 728 0 457 740 141 884 1024 1045 180 42 464 292 520 705 253
942 608 147 826 1292 780 893 1386 744 918 276 1054 669 1294 598 590 1217 171 1234 718 618 39 716 446
93 524 1229 666 840 943 1281 961 1259 102 151 522 1013 368 874 1233 1314 301 723 428 528 704 113 1391
1389 708 1253 692 286 563 1226 63 1082 914 523 209 1227 821 855 425 772 1065 1239 994 862 224 3 1109
1296 633 394 1104 839 1190 863 1214 810 617 273 1317 383 793 429 1177 1328 246 1299 374 1240 1135 12 15
905 99 868 941 591 1330 1080 812 1341 414 795 537 229 7 988 548 20 58 294 913 956 269 293 28
215 469 154 484 990 1141 1310 605 200 1107 1275 1383 1116 1367 859 930 188 854 556 1021 216 316 733 79
632 730 245 38 389 543 658 187 182 1152 916 597 580 362 1179 1078 1163 1345 899 602 1363 72 1180 973
560 1216 1258 922 1335 204 852 47 871 773 766 890 1191 419 1267 908 452 330 526 194 309 681 729 325
848 822 338 929 1362 115 750 346 409 1084 878 1318 1025 1134 1303 226 627 957 616 24 650 289 232 392
184 170 1029 365 1278 665 363 1359 1338 272 348 66 149 1128 726 499 738 211 40 841 168 1159 1095 126

58 and 24 are ROW and COLUMN values.

Arnab Nandy
  • 6,472
  • 5
  • 44
  • 50
0726
  • 315
  • 2
  • 13
  • `int **` is not a 2D array (aka matrix). It also cannot represent one, not point to one. If you need a 2D array, **use a 2D array**! – too honest for this site Jul 29 '16 at 20:44
  • Where's a 2D array? It is mentioned in the question title, but neither in the text, nor in the code. What is `ptr` and where is it initialized? – user3078414 Jul 29 '16 at 20:50
  • 2
    `fgets(NULL,NULL,f);` it's undefined behavior, at best. What are you trying to achieve? And what that `50` stands for? – Bob__ Jul 29 '16 at 21:02
  • @Olaf and user3078414 : if I do malloc and try to store data in 2D array fashion. Isnt it the same thing? m = (int **)malloc((ptr->row)*(ptr->col)*(sizeof(int))); I was trying to achieve the same using this part of the code. – 0726 Jul 30 '16 at 06:11
  • @Bob__ : that was jus to move to the next line. The fact that fscanf does not move the file pointer to next line. i used fgets just to move over to 2nd line of the file. 50 represents numbers of matrix. there are 50 matrix in that file. – 0726 Jul 30 '16 at 06:14
  • @0726 Well, that's not one of the many flaws of [scanf](http://en.cppreference.com/w/c/io/fscanf). _"...most conversion specifiers first consume all consecutive whitespace_ (and newlines, tabs) _... The conversion specifiers that do not consume leading whitespace, such as `%c`, can be made to do so by using a whitespace character in the format string: `scanf(" %c", &c);`"_ – Bob__ Jul 30 '16 at 11:41
  • "and try to store data in 2D array fashion" - The cooments should have told you your trial failed. I hate repeating myself, but **you don't have a 2D array, thus don't store 2D array alike!** There is enough to be found about howe to use 2D arrays and what they are/how to declare them. Learn! – too honest for this site Jul 30 '16 at 17:45

2 Answers2

2

If you validate each allocation, and each read, you will find the data you posted is only sufficient for 38 rows of 24 integers.

There are a number of issues in the code you posted. However, the overriding fault is the lack of validation of each critical step in your code. If any value is taken as input (whether entered by a user, or read from a file) and later used within your code, you must validate you have actually received the data you expect. In handling any input error, You also have the choice of whether to exit on error or to warn and substitute a default value in its place (e.g. nul or zero).

When declaring an instance of your struct to fill, makes sense to initialize the values (at least the first member so that the remainder are initialized to 0/nul).

Starting with opening your file, you do not want to read from the file unless, and until, you have validated it is open for reading, e.g.

    FILE *fp = fopen (argv[1], "r");

    if (!fp) {  /* validate file open for reading */
        fprintf (stderr, "error: file open failed '%s'.\n", argv[1]);
        return 1;
    }

Then in handling the initial lines in the file, it is probably a better idea to read the values into a buffer, and discard or parse the buffer as required. Something simple is all that is needed:

#define BUFSZ 256
...
    char buf[BUFSZ] = "";
    ...
    if (!fgets (buf, BUFSZ, fp)) {  /* throw away 1st line */
        fprintf (stderr, "error: empty file.\n");
        return 1;
    }

    if (!fgets (buf, BUFSZ, fp)) {  /* read 2nd line - str, row, col */
        fprintf (stderr, "error: reading data information.\n");
        return 1;
    }

    /* parse str, row, col & validate */
    if (sscanf (buf, " %19s %d %d", brovets.str, &brovets.row, &brovets.col)
               != 3) {
        fprintf (stderr, "error: unable to parse data information.\n");
        return 1;
    }

Since your member you wish to hold integer values in is a pointer-to-pointer-to-int, you need to declare row number of pointers, which you will allocate and assign to each a block of memory sufficient to hold col number of ints.

Here you also have a choice between malloc or calloc. There are certain advantages to either allocating with calloc (which both allocates and initializes to 0/NULL) For example, if you allocate 58 pointers, initialize to NULL and only read 38 rows worth of data, you can iterate over valid pointers to access the data you have has an alternative to keeping a counter to the number of rows filled. For example:

    /* allocate row number of pointers - initialize to zero w/calloc */
    if (!(brovets.arr = calloc (brovets.row, sizeof *brovets.arr))) {
        fprintf (stderr, "error: row pointer allocation failed.");
        return 1;
    }

    for (int i = 0; i < brovets.row; i++) { /* for each row */
        /* allocate storage for col ints & validate alloation */
        if (!(brovets.arr[i] = calloc (brovets.col, sizeof **brovets.arr))) {
            fprintf (stderr, "error: memory exhausted, row %d.\n", i);
            return 1;
        }
        for (int j = 0; j < brovets.col; j++)   /* read each value, validate */
            if (fscanf (fp, " %d", &brovets.arr[i][j]) != 1) {
                fprintf (stderr, "error: read of arr[%d][%d]\n", i, j);
                free (brovets.arr[i]);  /* free current row */
                brovets.arr[i] = NULL;  /* reset pointer for row to NULL */
                goto datadone;          /* leave input loop */
            }
    }
    datadone:;
    if (fp != stdin) fclose (fp);       /* close file if not stdin */

    printf ("\n name: %s\n rows: %d\n cols: %d\n\n",  /* output name, row, col */
            brovets.str, brovets.row, brovets.col);

    for (int i = 0; brovets.arr[i]; i++) {     /* output values stored in arr */
        for (int j = 0; j < brovets.col; j++) printf (" %4d", brovets.arr[i][j]);
        putchar ('\n');
    }

Finally, you need to track and free all of the memory you allocate. You do this by preserving a pointer to the starting address for each block you allocate. Here is where initializing your row-pointers NULL can help tell you which rows you need to free, you can iterate over the valid pointers until your first NULL is reached (or keep a counter).

    /* free allocated memory */
    for (int i = 0; brovets.arr[i]; i++) free (brovets.arr[i]);
    free (brovets.arr);

Putting it altogether, (and allowing input from the file specified as the first argument, or stdin by default) you could write your code in the following manner and insure you are only storing full rows of data, and handling any errors in a sane manner, and freeing all memory allocated when done.

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

#define BUFSZ 256

struct data {
    char str[20];
    int row, col;
    int **arr;
};

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

    char buf[BUFSZ] = "";
    struct data brovets = { .str="" };
    FILE *fp = argc > 1 ? fopen (argv[1], "r") : stdin;

    if (!fp) {  /* validate file open for reading */
        fprintf (stderr, "error: file open failed '%s'.\n", argv[1]);
        return 1;
    }

    if (!fgets (buf, BUFSZ, fp)) {  /* throw away 1st line */
        fprintf (stderr, "error: empty file.\n");
        return 1;
    }

    if (!fgets (buf, BUFSZ, fp)) {  /* read 2nd line - str, row, col */
        fprintf (stderr, "error: reading data information.\n");
        return 1;
    }

    /* parse str, row, col & validate */
    if (sscanf (buf, " %19s %d %d", brovets.str, &brovets.row, &brovets.col)
                != 3) {
        fprintf (stderr, "error: unable to parse data information.\n");
        return 1;
    }

    /* allocate row number of pointers - initialize to zero w/calloc */
    if (!(brovets.arr = calloc (brovets.row, sizeof *brovets.arr))) {
        fprintf (stderr, "error: row pointer allocation failed.");
        return 1;
    }

    for (int i = 0; i < brovets.row; i++) { /* for each row */
        /* allocate storage for col ints & validate alloation */
        if (!(brovets.arr[i] = calloc (brovets.col, sizeof **brovets.arr))) {
            fprintf (stderr, "error: memory exhausted, row %d.\n", i);
            return 1;
        }
        for (int j = 0; j < brovets.col; j++)   /* read each value, validate */
            if (fscanf (fp, " %d", &brovets.arr[i][j]) != 1) {
                fprintf (stderr, "error: read of arr[%d][%d]\n", i, j);
                free (brovets.arr[i]);  /* free current row */
                brovets.arr[i] = NULL;  /* reset pointer for row to NULL */
                goto datadone;          /* leave input loop */
            }
    }
    datadone:;
    if (fp != stdin) fclose (fp);       /* close file if not stdin */

    printf ("\n name: %s\n rows: %d\n cols: %d\n\n",  /* output name, row, col */
            brovets.str, brovets.row, brovets.col);

    for (int i = 0; brovets.arr[i]; i++) {     /* output values stored in arr */
        for (int j = 0; j < brovets.col; j++)
            printf (" %4d", brovets.arr[i][j]);
        putchar ('\n');
    }

    /* free allocated memory */
    for (int i = 0; brovets.arr[i]; i++) free (brovets.arr[i]);
    free (brovets.arr);

    return 0;
}

Example Use/Output

$ ./bin/array_brovets <dat/brovets.txt
error: read of arr[39][0]

 name: BOROVETS
 rows: 58
 cols: 24

 1090 1091 1060 1137  542   52  768  721  620  737  108  660 1069  789  356 1361 1255 1014 1171  494   57  565  122 1252
 1052 1118   17  490  594   22 1375 1070  571  236   82   87 1170  266  873 1340  529 1164  129  700  255 1041  972  927

You need to validate one more issue -- that you are correctly using and have freed all memory you have allocated. valgrind on Linux is the normal choice, but there are similar memory checkers for every platform. E.g.

Memory Error Check

$ valgrind ./bin/array_brovets <dat/brovets.txt
==2485== Memcheck, a memory error detector
==2485== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==2485== Using Valgrind-3.10.1 and LibVEX; rerun with -h for copyright info
==2485== Command: ./bin/array_brovets
==2485==
error: read of arr[39][0]

 name: BOROVETS
 rows: 58
 cols: 24

 1090 1091 1060 1137  542   52  768  721  620  737  108  660 1069  789  356 1361 1255 1014 1171  494   57  565  122 1252
 1052 1118   17  490  594   22 1375 1070  571  236   82   87 1170  266  873 1340  529 1164  129  700  255 1041  972  927
<snip>
==2485==
==2485== HEAP SUMMARY:
==2485==     in use at exit: 0 bytes in 0 blocks
==2485==   total heap usage: 39 allocs, 39 frees, 3,952 bytes allocated
==2485==
==2485== All heap blocks were freed -- no leaks are possible
==2485==
==2485== For counts of detected and suppressed errors, rerun with: -v
==2485== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 1 from 1)

There are multiple ways to approach this problem, all valid. A lot is left up to you. Look things over and let me know if you have any questions.

David C. Rankin
  • 81,885
  • 6
  • 58
  • 85
  • Ubi maior... There's one thing that bugs me, given the way you free memory, shouldn't be `calloc`ed `brovets.row` **+ 1** rows? – Bob__ Jul 30 '16 at 09:04
  • No, I think the confusion comes, in `if (fscanf (fp, " %d", &brovets.arr[i][j]) != 1)`, there if a read failure is experienced, the `row` is freed at that time and the pointer reset to `NULL`. That is the only place prior to the end `free` where one could have slipped by? And recall, with the `calloc` scheme, we need only iterate over *valid pointers* to have confidence we have freed all those that remain allocated at the end. – David C. Rankin Jul 30 '16 at 09:09
  • `calloc` does not set pointers to _null pointer_, but zeoes all bits. That is not necessarily the same as a null pointer (`NULL` is a macro with a _null pointer constant_ and does not apply here at all). Similar for floating point `0.0`. Only integers are guaranteed to have `0` with all bits zero. It differs in that aspect form the default initialisers. – too honest for this site Jul 30 '16 at 17:48
1

The member arr in OP's struct data is a pointer to a pointer to int, so I assume the intent is to store the matrix data into a dynamical allocated array of (rows) arrays of (columns) ints.

One issue is with this line:

m = (int **)malloc((ptr->row)*(ptr->col)*(sizeof(int)));

It seems to allocate enough space to store (number of rows) * (number of columns) ints, but If m is an int ** (considering the cast), we should first allocate (number of rows) pointers to int and then enogh space for all the matrix elements.

I'll show a possible implementation with a slightly modified input section and some input error checking added. I'd suggest to split the code (which I lazily put all in main()) in more useful and reusable functions. Please, note that the data posted by the OP doesn't completely fit a 58x24 matrix.

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

typedef struct {
        char str[20];
        int row, col;
        int **arr;
} data;

int main() {
    FILE *f = fopen("input.txt","r");
    if ( !f ) {
        fprintf(stderr, "Error: Unable to open input file.\n");
        exit(EXIT_FAILURE);
    }
    int num;
    data mat;

    if ( fscanf(f,"%d %19s%d%d", &num, mat.str, &mat.row, &mat.col) != 4 ) {
        fprintf(stderr, "Error: wrong file format.\n");
        exit(EXIT_FAILURE);
    }
    printf("%d\n%s %d %d\n",num,mat.str,mat.row,mat.col);

    mat.arr = malloc(mat.row * sizeof *mat.arr);
    if ( !mat.arr ) {
        fprintf(stderr, "Error: malloc failed.\n");
        exit(EXIT_FAILURE);
    }

    // now, I prefer one single allocation to keep data contiguous
    mat.arr[0] = malloc(mat.row * mat.col * sizeof *mat.arr[0]);
    if ( !mat.arr[0] ) {
        fprintf(stderr, "Error: malloc failed.\n");
        free(mat.arr);
        exit(EXIT_FAILURE);
    }
    int i, j;
    for ( i = 1; i < mat.row; ++i ) {
        mat.arr[i] = mat.arr[i - 1] + mat.col;
    }

    for ( i = 0; i < mat.row; ++i ) {
        for ( j = 0; j < mat.col; ++j ) {
            fscanf(f,"%d",&mat.arr[i][j]);
        }
    }

    for ( i = 0; i < mat.row; ++i ) {
        for ( j = 0; j < mat.col; ++j ) {
            printf("%5d", mat.arr[i][j]);
        }
        printf("\n");
    }

    fclose(f);
    free(mat.arr[0]);
    free(mat.arr);
    return EXIT_SUCCESS;
}
Bob__
  • 12,361
  • 3
  • 28
  • 42
  • dynamically allocating the space also works. Here's how: m = (int *)malloc(row*col*sizeof(int)); for(i=0;i – 0726 Jul 30 '16 at 10:23
  • Of course, just change `arr` to an `int *` in your `data` struct and then you can also use in that loop `*(m++)=atoi(line_buffer);` – Bob__ Jul 30 '16 at 11:23
  • That is not a 2D array as your answer implies. You can much easier allocated a 2D array with variable sizes using a VLA. That is standard C. – too honest for this site Jul 30 '16 at 17:51
  • @Olaf Quoting my own answer: _"The member arr in OP's struct data is a pointer to a pointer to int"_ . Where exactly do I imply a 2D array? In the next part of the statement were I use the words _"dynamical allocated array of (rows) arrays of (columns) ints."_ ? If you can rephrase the same concept in a more canonical and standardized way, please do it, I'm not an exepert and my english language skills are even worse. I've not used VLA because the OP didn't in the first place (as far as I can tell) so I thought VL2DA wouldn't be the best fit for an answer. VLA are optional in C11, AFAIK. – Bob__ Jul 30 '16 at 19:04
  • "assume the intent is to store the matrix data into a dynamical allocated array of (rows) arrays of (columns)" - You invoke the impression that approach is the only and correct for a 2D array. That is plain wrong! – too honest for this site Jul 30 '16 at 19:09