0

I'm trying to make a dynamic size matrix of 1-byte elements. To do so I defined the following function. The problem comes when I try to set the first "nrows" elements of the matrix to point the corresponding row (so I can do matrix[i][j]). It looks like that matrix[i] = matrix[nrows + i * single_row_elements_bytes]; doesn't work well enough (the program compiles but throws an core-segment-violation error). How can I make this work?

uint8_t **NewMatrix(unsigned nrows, unsigned ncols)
{

    uint8_t **matrix;
    size_t row_pointer_bytes = nrows * sizeof *matrix;
    size_t single_row_elements_bytes = ncols * sizeof **matrix;

    matrix = malloc(row_pointer_bytes + nrows * single_row_elements_bytes);

    unsigned i;

    for(i = 0; i < nrows; i++)
        matrix[i] = matrix[nrows + i * single_row_elements_bytes];

    return matrix;
}
C. P.
  • 41
  • 1
  • 1
  • 6

3 Answers3

0

I think there are a couple of problems with your code.

  • You can simplify this line:

    matrix = malloc(row_pointer_bytes + nrows * single_row_elements_bytes);
    

    to:

    matrix = malloc(row_pointer_bytes);
    

    Which allocates space for uint8_t* many rows in the matrix.

    The malloc() function only requires size_t amount of bytes needed to allocate requested memory on the heap, and returns a pointer to it.

    and that can simply be allocated by knowing how many rows are needed in the matrix, in this case nrows.

  • Additionally, your for loop:

    for(i = 0; i < nrows; i++)
        matrix[i] = matrix[nrows + i * single_row_elements_bytes];
    

    Doesn't allocate memory for matrix[i], since each row has n columns, and you need to allocate memory for those columns.

    This should instead be:

    for(i = 0; i < nrows; i++)
        matrix[i] = malloc(single_row_elements_bytes);
    
  • Another issue is how are allocating single_row_elements_bytes. Instead of:

    size_t single_row_elements_bytes = ncols * sizeof **matrix; //**matrix is uint8_t**
    

    This needs allocate uint8_t bytes for n columns, not uint8_t** bytes. It can be this instead:

    size_t single_row_elements_bytes = ncols * sizeof(uint8_t);
    

Having said this, your code will compile if written like this. This is an example I wrote to test the code.

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

uint8_t **NewMatrix(unsigned nrows, unsigned ncols);

int
main(int argc, char *argv[]) {
    uint8_t **returnmatrix;
    unsigned nrows = 2, ncols = 2;
    int i, j;

    returnmatrix = NewMatrix(nrows, ncols);

    for (i = 0; i < nrows; i++) {
        for (j = 0; j < ncols; j++) {
            printf("Enter number for row %d column %d: ", i+1, j+1);

            /* format speficier for uint8_t, from <inttypes.h> */
            if (scanf("%"SCNu8"", &returnmatrix[i][j]) != 1) {
                printf("Invalid 8 bit number.\n");
                exit(EXIT_FAILURE);
            }
        }
    }

    printf("\nYour matrix:\n");
    for (i = 0; i < nrows; i++) {
        for (j = 0; j < ncols; j++) {
            printf("%d ", returnmatrix[i][j]);
        }
        printf("\n");
    }

    /* Good to free at the end */
    free(returnmatrix);

    return 0;
}

uint8_t 
**NewMatrix(unsigned nrows, unsigned ncols) {
    int i;
    uint8_t **matrix;

    size_t row_pointer_bytes = nrows * sizeof * matrix;
    size_t column_row_elements_bytes = ncols * sizeof(uint8_t);

    matrix = malloc(row_pointer_bytes);

    /* Good to check return value */
    if (!matrix) {
        printf("Cannot allocate memory for %d rows.\n", nrows);
        exit(EXIT_FAILURE);
    }

    for(i = 0; i < nrows; i++) {
        matrix[i] = malloc(column_row_elements_bytes);
        if (!matrix[i]) {
            printf("Cannot allocate memory for %d columns.\n", ncols);
            exit(EXIT_FAILURE);
        } 
    } 

    return matrix;
}

Input:

Enter number for row 1 column 1: 1
Enter number for row 1 column 2: 2
Enter number for row 2 column 1: 3
Enter number for row 2 column 2: 4

Output:

Your matrix:
1 2
3 4

Compiled with:

gcc -Wall -o matrix matrix.c
RoadRunner
  • 25,803
  • 6
  • 42
  • 75
0

Apart from various bugs mentioned in another answer, you are allocating the 2D array incorrectly. You are actually not allocating a 2D array at all, but instead a slow, fragmented look-up table.

The correct way to allocate a 2D array dynamically is described at How do I correctly set up, access, and free a multidimensional array in C?. Detailed explanation about how this works and how array pointers work is explained at Function to dynamically allocate matrix.

Here is an example using the above described techniques, for your specific case:

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

void NewMatrix (size_t nrows, size_t ncols, uint8_t (**matrix)[nrows][ncols])
{
  *matrix = malloc ( sizeof (uint8_t[nrows][ncols]) );
}


int main (void) 
{
  size_t r = 3;
  size_t c = 4;

  uint8_t (*arr_ptr)[r][c];
  NewMatrix(r, c, &arr_ptr);
  uint8_t (*matrix)[c] = arr_ptr[0];

  uint8_t count=0;
  for(size_t i=0; i<r; i++)
  {
    for(size_t j=0; j<c; j++)
    {
      matrix[i][j] = count;
      count++;
      printf("%2."PRIu8" ", matrix[i][j]);
    }
    printf("\n");
  }

  free(arr_ptr);
}
Community
  • 1
  • 1
Lundin
  • 195,001
  • 40
  • 254
  • 396
  • why do you use size_t instead of unsigned? Thought it was just sizeof() return type (and also for malloc input) – C. P. Dec 21 '16 at 14:45
  • @C.P. It is a bit pedantic, but `size_t` is the most portable/correct type to use when describing the size of an array, or when iterating through an array. This is indeed related to `sizeof`, becase `sizeof(array)` is required to return the number of bytes used in an array, and therefore `size_t` must always be large enough to handle the size of the largest possible array on the given system. Suppose for example that you have a 16 bit system where unsigned int is 65535, but at the same time the system allows arrays that are larger than 64kb. – Lundin Dec 21 '16 at 15:54
  • Although the main reason for using `size_t` is probably to remain compatible with standard library functions, for example `memcpy` and `strlen`. – Lundin Dec 21 '16 at 15:58
  • I finally decided to implement the solution you proposed in "Function to dynamically allocate matrix", but, how could I set "matrix" as a input parameter to a function? – C. P. Dec 25 '16 at 13:01
  • e.g. to implement a function to print the matrix, taking the matrix and number of rows and columns as inputs – C. P. Dec 25 '16 at 13:04
0
    for (i=0;i<nrows;i++)
            matrix[i] = (uint8_t *)malloc(ncols * sizeof(uint8_t));

The below 2 lines have to be replaced by the above 2 lines

    for(i = 0; i < nrows; i++)
          matrix[i] = matrix[nrows + i * single_row_elements_bytes];

with (nrows + i * single_row_elements_bytes) as the allocation size, and with nrows = 5 and ncols =5, say, a total of 65 bytes are allocated. This include 40 bytes to store row pointers(assuming a 64bit pointer size) and rest 25 bytes to store contents of each r,c element. But the memory for column pointer (pointer to each element in that row) is not allocated.

So, dereferencing matrix[i][j] will seg fault.

Sibeesh Venu
  • 18,755
  • 12
  • 103
  • 140
vsj
  • 1
  • 1