transpose a matrix using c pointer, memory access error

Question

Without newly allocating memory to 'transposed' I tried to realloc on 'matrix' https://codepad.remoteinterview.io/WGIBSKENXV when I run it on web it works fine but on visual basic 2017, it gives me access violation error at double for loop part. The way that just allocates new memory to 'transposed' also works okay on web, but on visual basic 2017 it gives access violation error. What is wrong??

When I allocate memory in a defined function, should I free it in the main function? as with 'matrix'(the one that I allocated in main function)? Did I do it correctly?

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

 int** transpose (int **matrix, int m, int n);
 void printMatrix(int **matrix, int m, int n);

 int main (void){
 int rows, cols;
 int r, c, i;
 int **matrix;

 printf("Number of Rows : ");
 scanf("%d", &rows);
 printf("Number of Cols : ");
 scanf("%d", &cols);

 matrix = (int **)malloc(rows*sizeof(int*));
 for(i = 0; i < rows; i++){ 
 matrix[i] = (int*)malloc(cols*sizeof(int));
 }

 srand(2016);
 for( r = 0; r < rows; r++ ){
      for( c = 0; c < cols; c++ ){
           *(*(matrix + r)+c) = rand() % 99 + 1;
      }
 }

 printf("Matrix produced with seed number 2016\n");
 printMatrix(matrix, rows, cols);

 matrix = transpose(matrix, rows, cols);

 printf("Transposed Matrix\n");
 printMatrix(matrix, rows, cols);

         i = 0;
 while(matrix[i] != 0){
        free(matrix[i]);
        ++i;
 }
    free(matrix);
  /*
    i = 0;
    while(transposed[i] != 0){
         free(transposed[i]);
         ++i;
    }
    free(transposed);
  */   
    return 0;
 }
       //When I allocate memory in a defined function, should I free it in the main function? as with 'matrix'? Did I do it correctly?

 int** transpose (int **matrix, int rows, int cols){

 int i ,j;

 if( rows < cols ){
 for(i = 0; i < rows; i++){ 
 matrix[i] = (int*)realloc(matrix[i], rows*sizeof(int));
 }
 }
 else if(cols < rows){
 matrix = (int **)realloc(matrix, cols*sizeof(int*));
 }

   for (i = 0; i < rows; i++) {
    for (j = i +1 ; j < cols ; j++) {
     tmp= *(*(matrix + i) + j);
     *(*(matrix + i) + j) = *(*(matrix + j) + i);
     *(*(matrix + j) + i) = tmp;
    }
 }
      return matrix;

 // Without newly allocating memory to 'transposed' I tried to realloc on 'matrix'
 https://codepad.remoteinterview.io/WGIBSKENXV when I run it on web it works fine but on visual basic 2017, it gives me access violation error at double for loop part.


  /*
 int ** transposed;
 transposed = (int **)malloc(cols*sizeof(int*));
 for(i = 0; i < cols; i++){ 
 transposed[i] = (int*)malloc(rows*sizeof(int));
 }

 for (i = 0; i < rows; i++) {
    for (j = 0 ; j < cols; j++) {
     *(*(transposed + j )+ i )= *(*(matrix + i) + j);
     *(*(transposed + i) + j) = *(*(matrix + j) + i);
    }
 }     
 return transposed;
 */

 // Just allocating new memory to 'transposed' works okay on web https://codepad.remoteinterview.io/WGIBSKENXV, but on visual basic 2017 it gives access violation error. When I allocate memory in a defined function, should I free it in the main function? as with 'matrix'? Did I do it correctly?
 }     
 void printMatrix(int **matrix, int m, int n){
 int i, j;

 for( i = 0; i < m; i++ ){
 for( j = 0; j < n; j++ ){
 printf("%4d", *(*(matrix + i )+j) );
 }
 printf("\n");
 }

 }

There is NO need to cast the return of `malloc`, it is unnecessary. See: [**Do I cast the result of malloc?**](http://stackoverflow.com/q/605845/995714) — David C. Rankin, Jun 07 '17 at 06:42
when calling any of the heap allocation functions (malloc, calloc, realloc) 1) always check (!=NULL) the returned value to assure the operation was successful. 2) the returned type is `void*` so can be assigned to any pointer. Casting just clutters the code, making it more difficult to understand, debug, etc. 3) when calling `realloc()`, always assign the result to a temp variable then check that variable, and if not NULL, then assign to the target variable. Otherwise, when the function fails, the pointer to the original memory is lost, resulting in a memory leak — user3629249, Jun 08 '17 at 15:33
the line starting with: `https:` does not compile. Suggest modifying to: `//https:` — user3629249, Jun 08 '17 at 15:37
for ease of readability and understanding: 1) consistently indent the code. Indent after every opening brace '{'. unindent before every closing brace '}'. Suggest each indent level be 4 spaces. 2) separate code blocks (for, if, else, while, do...while, switch, case, default) via a single blank line. 3) separate functions by 2 or 3 blank lines (be consistent) 4) follow the axiom:: *only one statement per line and (at most) one variable declaration per statement.* — user3629249, Jun 08 '17 at 15:49
this statement: `*(*(matrix + r)+c) = rand() % 99 + 1;` will not do what you are expecting. the expression: `rand() % 99 + 1` will become: `rand() % 100`. Suggest: `(rand() % 99) + 1` — user3629249, Jun 08 '17 at 15:56
this statement: `srand(2016);` will always result in the same sequence of values from the `rand()` function. Suggest using something more appropriate like: `srand( time(void) );` — user3629249, Jun 08 '17 at 15:58
the functions (malloc, calloc, realloc) expect their parameter to be of type `size_t` not `int`, so the variable declarations for `rows`, `cols`, `r`, `c`, (and perhaps `i`) should be of type `size_t`, not `int`. The C 'implicit conversion' feature may save you, but should not be relied upon. — user3629249, Jun 08 '17 at 16:02
when calling any of the `scanf()` family of functions, always check the returned value (not the parameter values) to assure the operation was successful. — user3629249, Jun 08 '17 at 16:03
in the function: `transpose()`, the variable `tmp` is not declared. suggest replace this line: `tmp= *(*(matrix + i) + j);` with this line: `int tmp= *(*(matrix + i) + j);` — user3629249, Jun 08 '17 at 16:11
regarding this code block: `while(matrix[i] != 0) { free(matrix[i]); ++i; }` the allocation of the array of pointers for the `matrix` did not allocate an extra pointer, nor did it set such a pointer to NULL. so the loop will continue until some (random) area in memory is encountered that contains a NULL value. Suggest using something similar to: `for( size_t i=0; i — user3629249, Jun 08 '17 at 16:22
when transposing a matrix, allocate a new matrix, with the appropriate dimensions. Then copy the data from the old matrix to the new matrix, cell by cell. trying to perform the transpose 'in place' will result in corrupted data, possible seg fault events, corruption of the heap data structure, etc. — user3629249, Jun 08 '17 at 16:28
variable names (and function parameter names) should indicate `content` or `usage` (or better, both). parameter names like `m` and `n` and (perhaps) `r` and `c` are meaningless, even in the current context. — user3629249, Jun 08 '17 at 16:31

score 1 · Answer 1 · answered Jun 07 '17 at 03:35

 else if(cols < rows){
 matrix = (int **)realloc(matrix, cols*sizeof(int*));
 }

In this case you are reallocating matrix with lesser rows, but the loop that follows it still accesses it till rows

   for (i = 0; i < rows; i++) {
    for (j = i +1 ; j < cols ; j++) {
     tmp= *(*(matrix + i) + j);
     *(*(matrix + i) + j) = *(*(matrix + j) + i);
     *(*(matrix + j) + i) = tmp;
    }
 }

*(*(matrix + i) + j); when i > cols will result in access to invalid memory, as matrix is reallocated to cols number of addresses

transpose a matrix using c pointer, memory access error

1 Answers1