Partition a 1D char* into 2D char**

Question

There are a lot of questions about converting a 2D array into a 1D array, but I am attempting just the opposite. I'm trying to partition a string into substrings of constant length and house them in a 2D array. Each row of this 2D matrix should contain a substring of the initial string, and, if each row were to be read in succession and concatenated, the initial string should be reproduced.

I nearly have it working, but for some reason I am losing the first substring (partitions[0] -- length 8*blockSize) of the initial string (bin):

int main (void){
    char* bin = "00011101010000100001111101001101000010110000111100000010000111110100111100010011010011100011110000011010";
    int blockSize = 2; // block size in bytes
    int numBlocks = strlen(bin)/(8*blockSize); // number of block to analyze
    char** partitions = (char**)malloc((numBlocks+1)*sizeof(char)); // break text into block
    for(int i = 0; i<numBlocks;++i){
        partitions[i] = (char*)malloc((8*blockSize+1)*sizeof(char));
        memcpy(partitions[i],&bin[8*i*blockSize],8*blockSize);
        partitions[i][8*blockSize] = '\0';
        printf("Printing partitions[%d]: %s\n", i, partitions[i]);
    }
    for(int j=0; j<numBlocks;++j)
        printf("Printing partitions[%d]: %s\n", j,partitions[j]);
    return 0;
}

The output is as follows:

Printing partitions[0]: 0001110101000010
Printing partitions[1]: 0001111101001101
Printing partitions[2]: 0000101100001111
Printing partitions[3]: 0000001000011111
Printing partitions[4]: 0100111100010011
Printing partitions[5]: 0100111000111100
Printing partitions[0]: Hj
Printing partitions[1]: 0001111101001101
Printing partitions[2]: 0000101100001111
Printing partitions[3]: 0000001000011111
Printing partitions[4]: 0100111100010011
Printing partitions[5]: 0100111000111100

The construction of partitions in the first for loop is successful. After construction at read out, the string at partitions[0] contains garbage values. Can anyone offer some insight?

`malloc((numBlocks+1)*sizeof(char))` --> `malloc((numBlocks+1)*sizeof(char*))` — BLUEPIXY, Jan 24 '17 at 02:50
If the strings are to be fixed length, why use pointer to pointer in the first place? You could have declared a 2D array of fixed size. — Lundin, Jan 24 '17 at 07:49
I have to agree with @Lundin here, this code does not need any pointers. — RoadRunner, Jan 24 '17 at 12:08

score 0 · Answer 1 · edited May 23 '17 at 12:16

Their are a few problems with your code.

You are allocating **partitions incorrectly.

Instead of:

char** partitions = (char**)malloc((numBlocks+1)*sizeof(char)); /* dont need +1, as numblocks is enough space. */

You need to allocate space for char* pointers, not char characters.

instead, this needs to be:

char** partitions = malloc((numBlocks+1)*sizeof(char*));

Also read Why not to cast result of malloc(), as it is not needed in C.
malloc() needs to be checked everytime, as it can return NULL when unsuccessful.
Once finished with the space allocated, it is always good to free() memory previously requested by malloc(). It is important to do this at some point in the program.

Here is some code which shows this:

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

#define BLOCKSIZE 2
#define BLOCK_MULTIPLIER 8

int main(void) {
    const char *bin = "00011101010000100001111101001101000010110000111100000010000111110100111100010011010011100011110000011010";
    const size_t blocksize = BLOCKSIZE;
    const size_t multiplier = BLOCK_MULTIPLIER;
    const size_t numblocks = strlen(bin)/(multiplier * blocksize);
    const size_t numbytes = multiplier * blocksize;

    char **partitions = malloc(numblocks * sizeof(*partitions));
    if (partitions == NULL) {
        printf("Cannot allocate %zu spaces\n", numblocks);
        exit(EXIT_FAILURE);
    }

    for (size_t i = 0; i < numblocks; i++) {
        partitions[i] = malloc(numbytes+1);
        if (partitions[i] == NULL) {
            printf("Cannot allocate %zu bytes for pointer\n", numbytes+1);
            exit(EXIT_FAILURE);
        }
        memcpy(partitions[i], &bin[numbytes * i], numbytes);
        partitions[i][numbytes] = '\0';
        printf("Printing partitions[%zu]: %s\n", i, partitions[i]);
    }

    printf("\n");
    for(size_t j = 0; j < numblocks; j++) {
        printf("Printing partitions[%zu]: %s\n", j,partitions[j]);
        free(partitions[j]);
        partitions[j] = NULL;
    }

    free(partitions);
    partitions = NULL;

    return 0;
}

Which outputs non-garbage values:

Printing partitions[0]: 0001110101000010
Printing partitions[1]: 0001111101001101
Printing partitions[2]: 0000101100001111
Printing partitions[3]: 0000001000011111
Printing partitions[4]: 0100111100010011
Printing partitions[5]: 0100111000111100

Printing partitions[0]: 0001110101000010
Printing partitions[1]: 0001111101001101
Printing partitions[2]: 0000101100001111
Printing partitions[3]: 0000001000011111
Printing partitions[4]: 0100111100010011
Printing partitions[5]: 0100111000111100

score 0 · Answer 2 · edited May 23 '17 at 12:33

int numBlocks = strlen(bin)/(8*blockSize); // number of block to analyze
char** partitions = (char**)malloc((numBlocks+1)*sizeof(char)); // break text into block
for(int i = 0; i<numBlocks;++i){
    partitions[i] = (char*)malloc((8*blockSize+1)*sizeof(char));
    memcpy(partitions[i],&bin[8*i*blockSize],8*blockSize);
    partitions[i][8*blockSize] = '\0';
    printf("Printing partitions[%d]: %s\n", i, partitions[i]);
}

This all looks suspicious to me; it's far too complex for the task, making it a prime suspect for errors.

For reasons explained in answers to this question, void * pointers which are returned by malloc and other functions shouldn't be casted.
There's no need to multiply by 1 (sizeof (char) is always 1 in C). In fact, in your first call to malloc you should be multiplying by sizeof (char *) (or better yet, sizeof *partitions, as in the example below), since that's the size of the type of element that partitions points at.
malloc might return NULL, resulting in undefined behaviour when you attempt to assign into the location it points at.
Anything else (i.e. everything that isn't NULL) that malloc, calloc or realloc returns will need to be freed when no longer in use, or else tools such as valgrind (a leak detection program, useful for people who habitually forget to free allocated objects and thus cause memory leaks) will report false positives and lose part of their usefulness.
numBlocks, i, or anything else that's for counting elements of an array, should be declared as a size_t to follow standard convention (e.g. check the strlen manual, synopsis section to see how strlen is declared, noting the type of the return value is size_t). Negative values caused by overflows here will obviously cause the program to misbehave.
I gather you've yet to think about any excess beyond the last group of 8 characters... This shouldn't be difficult to incorporate.

I suggest using a single allocation, such as:

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

#define BLOCK_SIZE 8

int main(void) {
    char const *bin = "00011101010000100001111101001101000010110000111100000010000111110100111100010011010011100011110000011010";
    size_t bin_length = strlen(bin),
           block_count = (bin_length / BLOCK_SIZE)
                       + (bin_length % BLOCK_SIZE > 0); // excess as per point 6 above
    char (*block)[BLOCK_SIZE + 1] = malloc(block_count * sizeof *block);
    if (!block) { exit(EXIT_FAILURE); }
    for (size_t x = 0; x < block_count; x++) {
        snprintf(block[x], BLOCK_SIZE + 1, "%s", bin + x * BLOCK_SIZE);
        printf("Printing partitions[%zu]: %s\n", x, block[x]);
    }
    for (size_t x = 0; x < block_count; x++) {
        printf("Printing partitions[%zu]: %s\n", x, block[x]);
    }
    free(block);
    exit(0);
}

Partition a 1D char* into 2D char**

2 Answers2

Linked