0

So I'm trying to create a simple array 2D inside a struct, and to read the values of some coordinates that are in a file.

At this moment I've got all inside functions and i get a segmentation fault when i try to get the coordinates on the 'get_coords' function. If I write all the exactly same code on a 'int main' instead of using functions it works.

code:

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

            // structure
            typedef struct coordinates{
                double **a;
            } coord;

            // counts the nmbr of points 
            int count(){
                int i;
                int a1, a2, a3;
                FILE *fp;
                fp = fopen("abc.txt", "r+");
                while (fscanf(fp, "%d %d %d", &a1, &a2, &a3) != EOF){
                    i++;
                    if (feof(fp)){
                        break;          
                    }   
                }
                fclose(fp);
                return(i);
            }

            // creates new structure with the right size of memory allocated
            coord *newVector(size_t s){
                coord *v;
                int j;
                v = malloc(sizeof(coord));

                v->a = malloc(sizeof(double*)*3);

                for (j=0; j<3; j++){
                    v->a[j] = malloc(sizeof(double)*s);
                }

            }

            void get_coords(coord *points){
                int i=0;
                FILE *fp;
                fp = fopen("abc.txt", "r+");
                while (fscanf(fp, "%le %le %le", &points->a[i][0], &points->a[i][1], &points->a[i][2]) != EOF){
                    i++;
                }
                fclose(fp);
            }


            int main(){
                int i = 0, j=0;

                coord *points;

                i = count();

                points = newVector(i);

                get_coords(points);

                for (i=0; i<3; i++){
                        printf("%lf %lf %lf\n", points->a[i][0], points->a[i][1], points->a[i][2]);
                }
            }

abc.txt:

1 2 3
4 5 6
7 8 9

Thanks all for your help.

Cumps, Dylan.

PS: This is just a prototype for what i want.

gsamaras
  • 71,951
  • 46
  • 188
  • 305
  • 1
    Compile with all warnings and debug info (`gcc -Wall -Wextra -g` with [GCC](http://gcc.gnu.org/)...). Improve your code to get no warnings. Read [how to debug small programs](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/). Use [the `gdb` debugger](https://sourceware.org/gdb/current/onlinedocs/gdb/). Give an [MCVE] – Basile Starynkevitch Dec 19 '18 at 12:40
  • what is abc.txt file? – J...S Dec 19 '18 at 12:40
  • 1
    abc.txt file is just a txt with some coordinates (just an example of it) – Dylan Lopes Dec 19 '18 at 12:42
  • @BasileStarynkevitch it's almost an MCVE! – gsamaras Dec 19 '18 at 12:42
  • thanks @Basilestarynkevitch ill try – Dylan Lopes Dec 19 '18 at 12:42
  • 2D arrays (even if you call them simple) and input reading are two different non-trivial problems. Separate them. Try to solve one first, then start on the other. – Yunnosch Dec 19 '18 at 12:43
  • 1
    @Yunnosch thanks for your comment. As i said in question, the program works whe i've got everything in a 'int main', but once i separate the code into functions it stops working – Dylan Lopes Dec 19 '18 at 12:47
  • Would you like to show the working code before the refactoring? It would make finding the problematic change easier than looking for the problem in the shown code. – Yunnosch Dec 19 '18 at 12:49
  • Also, your comment does not seem to actually answer mine. Split the two parts. Solve each. Identify the one which makes the problem. Ask about that one. Demonstrate that one in a [mcve], exclude the other one from the question. – Yunnosch Dec 19 '18 at 12:50
  • @Yunnosch OP has already provided a very good attempt for an MCVE. – gsamaras Dec 19 '18 at 12:55
  • See [Correctly allocating multi-dimensional arrays](https://stackoverflow.com/questions/42094465/correctly-allocating-multi-dimensional-arrays) – Lundin Dec 19 '18 at 13:47
  • @gsamaras No has not. Code has been provided to demonstrate a problem with 2D array or reading. For "M" only the code whith the part which demonstrates the problem should be provided. Removing the rest is part of making the CVE minimal. – Yunnosch Dec 19 '18 at 15:46

2 Answers2

3

In count(), you are incrementing an uninitialized variable, invoking Undefined Behavior (UB).

Change this:

int count() {
  int i;

to this:

int count() {
  int i = 0;

newVector() is not returning the dynamic allocated memory.

Change this:

coord *newVector(size_t s) {
  ...
}

to this:

coord *newVector(size_t s) {
  ...
  return v;
}

After fixing these issues, you should see this output:

1.000000 2.000000 3.000000
4.000000 5.000000 6.000000
7.000000 8.000000 9.000000

Not the problem, but I would use %lf as the format specifiers in fscanf(), instead of %le.

Moreover, in count(), this return(i); is the same as this return i;. The parentheses are redundant.

TODO: Free the dynamically allocated memory (I assume that you skipped that part for providing the MCVE).


Pro-tip: Compile with warning flags enabled next time. They would have already found the issue for you in this case. In GCC for example, I would get this:

gsamaras@myPc:~$ gcc -Wall  main.c
main.c: In function ‘main’:
main.c:51:28: warning: unused variable ‘j’ [-Wunused-variable]
                 int i = 0, j=0;
                            ^
main.c: In function ‘newVector’:
main.c:37:13: warning: control reaches end of non-void function [-Wreturn-type]
             }
             ^

where the last warning is the second point in my answer.

gsamaras
  • 71,951
  • 46
  • 188
  • 305
  • You're right thanks, but still it doesn't solve my segmentation fault. Edited the question – Dylan Lopes Dec 19 '18 at 12:45
  • OK @DylanLopes, answer updated, problem solved. I rolled back your edit, since it invalidates my answer. :) Let me know if you have any problems.. – gsamaras Dec 19 '18 at 13:02
  • You are welcome @DylanLopes. I think this edit with compiler warnings was really helpful... PS: Not a great fail, just a mistake that might trigger new knowledge for you (like compiling with warnings enabled next time!) ;) PPS: Your MCVE was perfect, except from the fact that wasn't intended. – gsamaras Dec 19 '18 at 13:06
  • Thanks for your tips ;) now i've the problem that if i try read coordinates from this file: https://ufile.io/k1iap. i get a segmentation fault. if you can help please ;) PS: sorry for uploading the file, but it a really long one. – Dylan Lopes Dec 19 '18 at 13:33
  • I cannot open the file from my work @DylanLopes, it's not a trusted file. How many columns, and how many rows? Because in your code, you have 3 hardcoded. – gsamaras Dec 19 '18 at 13:37
  • 19352 rows and 3 columns, i've hardcoded 3 columns, but unlimited rows right? – Dylan Lopes Dec 19 '18 at 13:41
  • 1
    Ok i got it wrong, i was allocatting memory for unlimited collumns instead of rows, i just changed that and it works, that for everything @gsamaras – Dylan Lopes Dec 19 '18 at 13:47
  • No @DylanLopes, as I explain in my [2D dynamic array(C)](https://gsamaras.wordpress.com/code/2d-dynamic-array-c/), you are allocating dynamically memory for 3 rows and `s` columns. Glad I helped! – gsamaras Dec 19 '18 at 13:47
0

You forgot to return v in newVector function:

        // creates new structure with the right size of memory allocated
        coord *newVector(size_t s){
            coord *v;
            int j;
            v = malloc(sizeof(coord));

            v->a = malloc(sizeof(double*)*3);

            for (j=0; j<3; j++){
                v->a[j] = malloc(sizeof(double)*s);
            }

            return v;

        }
garlix
  • 576
  • 9
  • 26