For-loop - Structure

Question

I have this part of code:

for(i = 0; i < matrix->y; i++)
{
  printf("3.%i - %i %i\n", i, matrix->y, matrix->x);//write correct "3.0 - 3 4"
  for(j = 0; j < matrix->x; j++)
  {
    printf("N - %i %i\n", matrix->y, matrix->x);//never write anything
    fscanf(input, "%i", &grid[i][j]);
  }
  printf("3.%i - %i %i\n", i, matrix->y, matrix->x);//write wrong "3.0 - 3 0"
}

first printf outputs y=3 and x=4, but it never goes inside loop, it never reach the second printf inside for-loop. When I write the same printf from the first line after for-loop, it tels me y=3 and x=0. Where I have done mistake.

Thanks.

Edit

Code is writen as it is. No lines skipped.

Matrix definition

typedef struct matrix
{
  int x;
  int y;
  int ** grid;
} Matrix;

i and j is defined by

int i, j;

Whole function

Matrix * loadMatrix(char * filename)
{
  Matrix * matrix;
  FILE * input;
  input = fopen(filename, "r");
  if (input == NULL)
    printError (ERR_READFILE);
  else
  {
    fscanf(input, "%i", &(matrix->y));
    fscanf(input, "%i", &(matrix->x));
    int i, j;
    int grid[matrix->y][matrix->x];
    matrix->grid = grid;
    printf("2 - %i %i\n", matrix->y, matrix->x);
    for(i = 0; i < matrix->y; i++)
    {
      printf("3.%i - %i %i\n", i, matrix->y, matrix->x);
      for(j = 0; j < matrix->x; j++)
    {
    printf("N - %i %i\n", matrix->y, matrix->x);
    fscanf(input, "%i", &grid[i][j]);
    }
    printf("3.%i - %i %i\n", i, matrix->y, matrix->x);
    fclose(input);
  }
  return matrix;
}

You should let us see the definition of your `matrix` structure. — Constantinius, Oct 04 '11 at 15:26
If you show us the rest of the code then we can work out where you set `x=0` — David Heffernan, Oct 04 '11 at 15:28
I'm thinking something along the lines of `j` aliasing `matrix->x` — pmg, Oct 04 '11 at 15:32
wild guess, did you write in your code before what we see the next line: "matrix = &j"? — Roee Gavirel, Oct 04 '11 at 15:39
If you repeat the first `printf()`, do you get the same data printed both times? If not, it hints that `matrix` was a pointer to a local variable of a function that has since returned. — Jonathan Leffler, Oct 04 '11 at 15:39
Any chance that `matrix` is not really pointing to a `Matrix`, but rather some random part of memory, and you just get lucky the first time? Do any external threads have access to `matrix`? — Brian McFarland, Oct 04 '11 at 15:57
No really show us the code. Stop showing fragments. Post a complete program. — David Heffernan, Oct 04 '11 at 15:59
it reads correct values x and y from given file(tried more values) — user978734, Oct 04 '11 at 16:01
@BrianMcFarland: great guess. Real nice case if undefined behavior IMO, the behavior is surprising. — Mat, Oct 04 '11 at 16:23
`%i` is not a valid (standard) format for `fscanf`. It can be used with the `printf` family only. You probably meant `%d`. — Sander De Dycker, Oct 04 '11 at 16:44
Moral of the story is to post minimal yet complete code. Now you should accept one of the answers. — David Heffernan, Oct 04 '11 at 17:54

score 3 · Accepted Answer · edited May 23 '17 at 11:55

3

Your program's behavior is undefined because you're not allocating storage for matrix. So anything could happen.

Add this somewhere after you have declared matrix (just before the fscanf lines seems optimal):

matrix = malloc(sizeof(Matrix));

and change the line where you declare matrix to:

Matrix *matrix = NULL;

(otherwise you'll return garbage if the fopen fails.)

Second problem is that you're then also failing to allocate proper storage for matrix->grid. You can't keep a pointer to a local variable after the function has returned.

You'll need to create that storage with malloc or calloc too. See this question: How do I work with dynamic mutli-dimensional arrays in C for an example of how to do that.

You'll also need to take care to free both of those once you're finished using them.

Don't forget to #include <stdlib.h>, which is required for malloc and free.

edited May 23 '17 at 11:55

Community

1
1

answered Oct 04 '11 at 16:06

Mat

202,337
40
393
406

If you add that just after the declaration of `matrix`, if won't work in pre-C99. – svick Oct 04 '11 at 16:10
Gah, you're right, _and_ it wasn't a good idea to put it there anyway. – Mat Oct 04 '11 at 16:18
+1 for the `Matrix matrix = NULL`. Because initializing pointers to NULL will (usually) result in a segfault if you attempt to dereference them. Which makes this kind of bug result in well defined, easy to debug hard faults rather than bizarre, undefined behavior. – Brian McFarland Oct 05 '11 at 13:56

score 1 · Answer 2 · answered Oct 04 '11 at 16:04

1

Where I have done mistake?

You haven't allocated space to access through your matrix pointer.

answered Oct 04 '11 at 16:04

pmg

106,608
13
126
198

score 1 · Answer 3 · answered Oct 04 '11 at 16:09

In your code, you never initialize matrix, which means it can point to anything. That can cause access violation errors (if you're lucky) or weird errors (if you aren't). To initialize it, use malloc:

matrix = malloc(sizeof(Matrix));

Don't forget to deallocate the memory when you don't need the Matrix anymore by using free.

You should also check that malloc didn't return NULL, which can happen when there isn't enough free memory available.

For-loop - Structure

3 Answers3