1

I'm working on a project and I'm kinda stuck at a problem. I'm trying to read a file that contains a number in the first line that gives the number of rows, and then following the matrix of integers, separated by spaces.

I want to make a pointer in the main, then call the function with the pointer as parameter, the function should read the first number in the txt file, then create a 2d array with malloc, then read the matrix in the textfile and return. but either i get it so the function can allocate and read the matrix, but then i have something wrong with the pointer when calling the function so i cant use the allocated and read stuff in the main, or im getting errors when trying to call by reference and allocate stuff in the function then.

void readjobs(FILE* fp, int ***array, int linesToRead, int facilityCount) {
    int ch = 0;
    int rows = 0;
    while ((ch = fgetc(fp)) != '\n')
    {
        rows = ch - 48;
        //rows = rows * 10 + (ch - 48);
    }

    if (rows > linesToRead) rows = linesToRead;

    *array = (int**)malloc(rows * sizeof(int*));
    for (int i = 0; i < rows; i++) {
        /* size_y is the height */
        *array[i] = (int*)malloc(facilityCount * sizeof(int));
    }
    int i = 0, j = 0;
    while ((ch = fgetc(fp)) != EOF)
    {
        if (ch == '\n')
        {
            i++;
            printf("\n");
        }
        else if (ch == ' ')
        {
            j++;
            printf("%i ", *array[i][j]);
        }
        else    //wenn es ne nummer ist
        {
            *array[i][j] = (*array[i][j] * 10) + (ch - 48);
        }
    }
}

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

    int facilities_count = -1;
    int jobs_count = -1;
    int **jobs = NULL;
    FILE *fp;   //Zeiger für Datei
    fp = fopen("jobs.txt", "r");    //Dateizugriff, Datei als read 

    if (fp == NULL) {   //falls die Datei nicht geoeffnet werden kann
        printf("Datei konnte nicht geoeffnet werden!!\n");
    }
    else {  //Datei konnte geoeffnet werden
        printf("Datei ist lesbar\n");

        readjobs(fp, &jobs, 6, 6);

        if (jobs == NULL)printf("nullpionter");
        else {
            for (int i = 0; i < 6; i++) {
                printf("\n");
                for (int j = 0; j < 6; j++) {
                    printf("%x ", jobs[i][j]);
                }
            }
        }
        fclose(fp); //Dateizugriff wieder freigeben
    }

    MPI_Finalize();
    getchar();
    return 0;
}

Textfile example:

6
3 2 2 1 5 4
1 1 3 4 2 0
1 2 3 4 5 1
3 4 2 0 1 5
1 0 5 2 3 4
4 0 1 3 5 2

the first number "6" in this case is how many lines, and the rest is the matrix to be read

klutt
  • 30,332
  • 17
  • 55
  • 95
tiamat
  • 13
  • 3
  • Can you give us an exemple of input and what is the expected output ? – Stargateur Aug 19 '18 at 23:17
  • the jobs.txt would be like 1. line a single number, for example "6", and then 6 lines with 6 integers seperated by spaces. – tiamat Aug 19 '18 at 23:39
  • Why aren't you using `fscanf()`? – Iharob Al Asimi Aug 20 '18 at 00:23
  • 3
    `*array[i][j]` doesn't do what you think it means. I think you mean `(*array)[i][j]`. The array subscripting operators have a higher precedence than the indirection / dereference operator. – Grant Sanders Aug 20 '18 at 00:31
  • 1
    Consider returning a pointer to your newly allocated and filled block of memory -- becoming a *3-Star Programmer* in C isn't a compliment `:)` Also, don't use *magic numbers* in your code, e.g. `ch - 48` is better written `ch - '0'`, consider a `#define FOO 10` and use `FOO` instead of `10` throughout your code (it provides a single point of change, rather than picking through your code) – David C. Rankin Aug 20 '18 at 02:00
  • 1
    Instead of this "three star programming", consider [Correctly allocating multi-dimensional arrays](https://stackoverflow.com/questions/42094465/correctly-allocating-multi-dimensional-arrays) instead. – Lundin Aug 20 '18 at 13:41
  • the posted code is missing a few key statements, like the `#include` statements. Are you expecting us to guess as to which header files your actual code includes? – user3629249 Aug 22 '18 at 16:28
  • OT: in C, the heap allocation functions: `malloc` `calloc` `realloc` have a return type of `void*` so can be assigned to any pointer. Casting just clutters the code, making it more difficult to understand, debug, etc. – user3629249 Aug 22 '18 at 16:36
  • regarding: `int main(int argc, char** argv) {` When the passed parameters to `main()` are not going to be used, to avoid warning messages about unused parameters, use the signature: `int main( void )` When compiling, always enable the warnings, then fix those warnings. ( for `gcc`, at a minimum use: `-Wall -Wextra -Wconversion -pedantic -std=gnu17` ) Note: other compilers use different options to obtain the same results – user3629249 Aug 22 '18 at 16:38
  • OT: the function: `printf()` is rather `expensive` in CPU cycles. when wanting to output only a newline, suggest: `puts("");` – user3629249 Aug 22 '18 at 16:41
  • OT: regarding: `*array = (int**)malloc(rows * sizeof(int*));` the parameter to any of the heap allocation functions needs to have type: `size_t` (usually an unsigned long int)` passing an `int` to the function results in a warning message about implicit conversion from `int` to `size_t` can result in changing the sign of the value – user3629249 Aug 22 '18 at 16:44

1 Answers1

2

Your main problem

I compiled your code with debugging on:

$ cc -g mat.c -Wall -Wextra

Ran it in debugger:

$ gdb a.out
(gdb) run
Starting program: /tmp/a.out 
Datei ist lesbar

Program received signal SIGSEGV, Segmentation fault.
0x00005555555548ff in readjobs (fp=0x555555756010, array=0x7fffffffe740, linesToRead=6, facilityCount=6)
    at mat.c:18
18          *array[i] = (int*)malloc(facilityCount * sizeof(int));

Ok, so it crashes at this line:

*array[i] = (int*)malloc(facilityCount * sizeof(int));

Doing so is always a good idea to find out what the problem is. What could be the cause? Unfortunately it is not trivial in this case. Unless you really understand pointers. *array[i] is not what you want. You want (*array)[i].

Remember that x[i] is just a shortcut for *(x+i). Furthermore, [] has higher priority than *. So *x[i] = *(x[i]) = *(*(x+i)), but what you want is (*x)[i] = *((*x) + i) which clearly is not the same thing.

Other stuff

I would definitely extract the creation of the matrix, like this:

int ** createMatrix(int rows, int columns) {
    printf("%d %d\n", rows, columns);
    int **ret;
    ret = malloc(rows * sizeof *ret);
    if(!ret) { perror("Error: "); exit(EXIT_FAILURE); }
    for(int i=0; i<columns; i++) {
        ret[i] = malloc(columns * sizeof (*ret)[0]);
        if(!ret[i]) { perror("Error: "); exit(EXIT_FAILURE); }
    }
    return ret;
}

and then in your code:

*array = createMatrix(rows, facilityCount);
// Don't trust my code. Check the pointer.
if(!array) { perror("Error: "); exit(EXIT_FAILURE); } 

Remember to ALWAYS check if malloc succeeded.

And your way of reading the numbers is very weird. If you are a beginner who came up with this method on your own, that's actually pretty impressive, but it's not a good method. Read about fscanf and getline.

klutt
  • 30,332
  • 17
  • 55
  • 95
  • I understood what you said and changed my programm accordingly. I also implemented your create function. i fixed another j = 0; that was missing in the reading loop, and now it is working fine, thx vry much. also i used fscanf to read in the numbers. Other question, do i have to do anything to free the space i made with malloc? like "free(jobs);" in the main before exiting? – tiamat Aug 20 '18 at 02:15
  • Glad it worked. In your particular case it will cause no problems to not free, but yes in general EVERY malloc should have a corresponding free. This can be a bit tricky when you are using pointer to pointer techniques. If you are happy with the answer above, please mark it as accepted as described [here](https://stackoverflow.com/help/someone-answers) – klutt Aug 20 '18 at 02:25