CODEWITHSUNDEEP
cmultidimensional-arraymemory-management
ant2009
ant2009

Reputation: 22476

Allocating dynamic 2D char array

gcc 4.6.2 c89

Allocating memory for a 2D array and filling with characters.

However, I don't seem to be filling as when I print nothing is displayed.

Am I doing something wrong here?

char **attributes = NULL;

/* TODO: Check for memory being allocated */
attributes = malloc(3 * sizeof(char*));
int i = 0;
int k = 0;

for(i = 0; i < 3; i++) {
    for(k = 0; k < 5; k++) {
        sdp_attributes[i] = malloc(5 * sizeof(char));
        sdp_attributes[i][k] = k;
    }
}

for(i = 0; i < 3; i++) {
    for(k = 0; k < 5; k++) {
        printf("attributes[i][k] [ %c ]\n", attributes[i][k]);
    }
}

Many thanks for any advice,

Upvotes: 1

Views: 5588

Answers (4)

Jonathan Leffler
Jonathan Leffler

Reputation: 753415

You probably want:

for (i = 0; i < 3; i++)
{
    attributes[i] = malloc(5 * sizeof(char));
    for (k = 0; k < 5; k++)
    {
        attributes[i][k] = k;
    }
}

This ignores error checking on the allocation.

It also fixes the name of the array to match the declaration, but your code either wasn't compiling (don't post non-compiling code unless your question is about why it doesn't compile!) or you have another variable called sdp_attributes declared somewhere which you weren't showing us.

Your code was leaking a lot of memory. Each time around the k-loop, you allocated a new array of 5 characters and stored the pointer in attributes[i] (or sdp_attributes[i]), storing the new pointer over what was there before, so you overwrote the value of the first 4 pointers. You could not possibly free the first four items - they were lost irretrievably. Also, on the last iteration, you initialized the 5th element of the final array, but the previous 4 were not initialized and therefore contained indeterminate garbage.

Also, in your printing loop, the values in the array are control characters ^@, ^A, ^B, ^C and ^D; these do not necessarily print well with %c (especially not ^@, which is also known as NUL or '\0'). The printf() statement might be better written as:

    printf("attributes[%d][%d] [ %d ]\n", i, k, attributes[i][k]);

This prints the array indexes (rather than simply the characters [i][k] for each entry), and prints the control characters as integers (since the char values are promoted to int when passed to printf()) rather than as control characters.

(It's also more conventional to use i and j for a pair of nested loops, and i, j, and k for triply nested loops, etc. However, that's a very minor issue.)

Upvotes: 3

Sangeeth Saravanaraj
Sangeeth Saravanaraj

Reputation: 16587

The correct way to allocate and assign elements to 2d array is as follows (but this is a int array, you can try and change it for char array):

One thing to note: As mentioned by @Mysticial, you should add/subtract '0' to your int value to get the char value when using ASCII character set (remember our itoa() functions!). 

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

int main()
{
        int row, column;
        int **matrix;
        int i, j, val;

        printf("Enter rows: ");
        scanf("%d", &row);
        printf("Enter columns: ");
        scanf("%d", &column);

        matrix = (int **) malloc (sizeof(int *) * row);
        for (i=0 ; i<row ; i++) 
                matrix[i] = (int *) malloc (sizeof(int) * column);

        val=1;
        for (i=0 ; i<row ; i++) {
                for (j=0 ; j<column; j++) {
                        matrix[i][j] = val++;
                }
        }

        for (i=0 ; i<row ; i++) {
                for (j=0 ; j<column; j++) {
                        printf("%3d  ", matrix[i][j]);
                }
                printf("\n");
        }

        for (i=0 ; i<row ; i++) 
                free(matrix[i]);
        free(matrix);

        return 0;
}

Few points to note:

  1. error handling should be added for malloc()
  2. malloc()'ed memory must be free()'ed

Upvotes: 1

ob_dev
ob_dev

Reputation: 2838

for(i = 0; i < 3; i++) {
    for(k = 0; k < 5; k++) {
        sdp_attributes[i] = malloc(5 * sizeof(char));
        sdp_attributes[i][k] = k;
    }
}

Your erasing the allocated memory every time you loop in the inner most loop. Here is a correct version.

for(i = 0; i < 3; i++) {
    sdp_attributes[i] = malloc(5 * sizeof(char));
    for(k = 0; k < 5; k++) {
        sdp_attributes[i][k] = k;
    }
}

And you should fix your declaration:

 attributes = malloc(3 * sizeof(char*));

to

   sdp_attributes = malloc(3 * sizeof(char*));

Don't forget to free up all the memory allocated

for(i = 0; i < 3; i++)
{
   free(sdp_attributes[i]);
}
free(sdp_attributes);

Upvotes: 1

Mysticial
Mysticial

Reputation: 471209

Two major issues:

First Issue:

for(i = 0; i < 3; i++) {
    for(k = 0; k < 5; k++) {
        sdp_attributes[i] = malloc(5 * sizeof(char));

You are reallocating sdp_attributes[i] at each iteration of the inner loop - thereby overwriting it each time. You probably wanted this instead:

for(i = 0; i < 3; i++) {
    sdp_attributes[i] = malloc(5 * sizeof(char));
    for(k = 0; k < 5; k++) {

Second Issue:

sdp_attributes[i][k] = k;

You are basically writing the lower ascii characters. Most of them are not printable. Something like this might do what you want:

sdp_attributes[i][k] = k + '0';

Upvotes: 6

Related Questions