1

I'm trying to manage 4D dynamic-allocated array. The code in the else statement give me the error. If I don't include the else statement, the code goes right, but is this a good practice?

int**** datind = (int****) malloc(nRow * sizeof(int***));
for (size_t i = 0; i < nRow; i++) {
    datind[i] = (int***) malloc(nCol * sizeof(int**));
}
for (size_t i = 0; i < nCol; i++) {
    for (size_t j = 0; j < nRow; j++) {
        datind[j][i] = (int**) malloc(1 * sizeof(int*));
    }
}

while ( fscanf(fp1, "%d %*c %d %*c %zu", &row, &col, &n_value) != EOF ) {
    if (n_value > 0) {
       datind[row-1][col-1] = (int**) realloc(datind[row-1][col-1], n_value * sizeof(int*));
       for (size_t i = 0; i < n_value; i++) {
           datind[row-1][col-1][i] = (int*) realloc(datind[row-1][col-1][i], 6 * sizeof(int));
           for (size_t j = 0; j < 6; j++) {
               fscanf(fp1,"%d%*c", &datind[row-1][col-1][i][j] );
           }
    } else {
       datind[row-1][col-1][0] = (int*) realloc(datind[row-1][col-1][0], 1 * sizeof(int));
       datind[row-1][col-1][0][0] = -1;
    }
}

Similar code for 3D array, i.e starting with int***, works well.

gallog
  • 11
  • 2
  • 2
    (1) `fscanf()` can return values other than `EOF` if errors occur. If that happens, some of the values being read are not modified - and if they are uninitialised, using their values causes undefined behaviour. (2) You're not doing any range checking on `row` and `col`. If any are zero, using `datind[row-1][col-1]` gives undefined behaviour. (3) The undefined behaviour is not only in the `else`. (4) Undefined behaviour can seem to work, but that doesn't make it right. – Peter Jun 11 '19 at 15:29
  • 1
    You are not checking the return values of your allocations and reallocations to verify that they succeeded. That's probably not causing the specific error you describe, but it may bite you later. – John Bollinger Jun 11 '19 at 15:31
  • Related: https://stackoverflow.com/questions/21006707/proper-usage-of-realloc Also: https://stackoverflow.com/questions/42094465/correctly-allocating-multi-dimensional-arrays – Bob__ Jun 11 '19 at 15:32
  • In any event, in order to answer debugging questions, we ordinarily need a [mre]. The code presented could be wrapped fairly trivially into a complete program, but if we do that ourselves then we cannot be certain to capture any relevant detail that you may have omitted. Also, it may be that the input being fed to the program is a factor, and we don't know what that is unless you tell us. – John Bollinger Jun 11 '19 at 15:37
  • It's really hard to count the number of `*` in a long string. It's much easier to read the code if you write `int**** datind = malloc(nRow * sizeof *datind);` . (Omit the cast, whose only purpose is to suppress a compiler warning that you will want to to see, and compute the size of an object rather than a type.) – William Pursell Jun 11 '19 at 15:58
  • @WilliamPursell thanks for the hint, improving legibility is fundamental practice while I'm going deep in learning C. But if I omit the cast, I will get an error, not a warning. – gallog Jun 12 '19 at 16:56
  • @Bob__ Related posts are really interesting for better understanding dynamic memory alloc. I'm going to study them further. At a glance, it seems that using `malloc()` slows the execution and this is a crucial point because I'm trying to replicate a MATLAB script in a low level language to speed up calculation. At the same time, I can't allocate statically `datind` because full size exceeds stack. – gallog Jun 12 '19 at 17:08
  • If you get an error when you omit the cast of malloc, you have a problem. Perhaps you are compiling your code as C++. – William Pursell Jun 12 '19 at 17:36
  • @WilliamPursell Yes, of course. But I need to compile as C++ for libraries. I'm going to correct the tag of the question. – gallog Jun 13 '19 at 00:20
  • Oh gawd - we have a 4-star C hero here. http://wiki.c2.com/?ThreeStarProgrammer – Michael Dorgan Jun 13 '19 at 00:34
  • @MichaelDorgan Sorry if hurt you with my 4-star code, now I downgraded it to 2-star. – gallog Jun 13 '19 at 00:59
  • The `%*c` are very suspicious – M.M Jun 13 '19 at 01:25

1 Answers1

0

SOLVED! Ok, this works. I included the hint in comments for improving quality of my code. No need to check if row and col are equal to 0, because I know that they can go from 1 to 197. Thank you.

int **datind[(2*nx-1)][(2*ny-1)];

while ( fscanf(fp1, "%d %*c %d %*c %zu", &row, &col, &n_value) != EOF ) {

    if (n_value > 0) {

        datind[row-1][col-1] = (int**) malloc(n_value * sizeof(*datind));
        if (datind[row-1][col-1] == NULL) {
            printf("Memory allocation failed.\n");
            return EXIT_FAILURE;
        }

        for (size_t i = 0; i < n_value; i++) {
            datind[row-1][col-1][i] = (int*) malloc(6 * sizeof(*datind[row-1][col-1]));
            if (datind[row-1][col-1][i] == NULL) {
                printf("Memory allocation failed.\n");
                return EXIT_FAILURE;
            }

            for (size_t j = 0; j < 6; j++) {
                fscanf(fp1,"%d%*c", &datind[row-1][col-1][i][j] );
            }

        }
    } else {
        datind[row-1][col-1] = NULL;
    }

}
gallog
  • 11
  • 2