Rewrite C++ class in C

Question

So, I am having some trouble rewriting a C++ class I made in C.

The C++ class has some private attributes:

int grid_width;
int grid_height;
const int group_width = 2;
const int group_height = 4;
std::vector<int> buffer;

It is initialized like so:

grid::grid(int width, int height) {
            this->grid_width = width;
            this->grid_height = height;
            buffer.resize(this->grid_width / this->group_width * this->grid_height / this->group_height, 0);
    
}

It also comes with a clear function like so:

void grid::clear() {
    // get_buffer_size returns elements in the buffer vector
    for (int i = 0; i < get_buffer_size(); ++i) {
        buffer[i] = 0x00;
    }
}

Now, my attempt to rewrite this in C looks somewhat like this:

typedef struct
{
    int width;
    int height;
    int *buffer;
} grid;

grid *grid_new(int grid_width, int grid_height)
{
    if ((grid_width % 2 != 0) || (grid_height % 4 != 0))
        return NULL;

    int group_height = 4;
    int group_width = 2;

    grid *p_grid = calloc(grid_width / group_width * grid_height / group_height, sizeof(int));
    p_grid->width = grid_width;
    p_grid->height = grid_height;

    return p_grid;
}

void grid_free(grid *p_grid)
{
    free(p_grid->buffer);
    free(p_grid);
}

void grid_clear(grid *g)
{
    // ToDo: Iterate over all elements in the buffer
    int elements = sizeof(g->buffer) / sizeof(int);
    printf("Elements: %i", elements);
}

But for some reason, the amount of elements in my C code is always 2? Does anyone know where I am messing up?

If the grid is initialized with 4 and 8, the expected buffer size should be 4, not 2. If it would be initialized with 10 and 24, the expected size would be 30, but it still remains 2 in my C example.

`sizeof(g->buffer)` -- This does not do what you believe it does. This does not give you the number of bytes allocated. This is the case regardless if you are using `C++` or `C`. — PaulMcKenzie, Jun 28 '20 at 17:37
If you need to know how many elements the pointed to array has you need to store that value somewhere. `std::vector` does that internally, in C you have to do it yourself (unless you use a library that has a nice implementation of a dynamic array) — UnholySheep, Jun 28 '20 at 17:41
Once you fix the issue @PaulMcKenzie mentioned you'll have other problems. Note that you only have one `calloc` but two `free`s. You also never initialize `p_grid->buffer`. I assume you intended that `calloc` to be allocating space for `buffer`, but that's not how you're using it. — Miles Budnek, Jun 28 '20 at 17:44
Also, believe it or not, `C` allows you to return `struct` by value, just like C++. There is no need to return a `grid *` for the `grid_new` function. Declare a local `grid`, initialize it properly, and return it.. `grid grid_new(int grid_width, int grid_height)`. Doing things that way would also probably prevented the error pointed out by @MilesBudnek — PaulMcKenzie, Jun 28 '20 at 17:50
I understood that in my C code, I was just getting the size of the pointer instead of the buffer. I tried to run the example you provided but it results in a segfault, and I am not entirely sure what is causing it. I'll try to do some further debugging. — Kyu96, Jun 28 '20 at 18:03
[There is no issue with the code here](https://ideone.com/jp9YJj). — PaulMcKenzie, Jun 28 '20 at 18:06

score 1 · Answer 1 · answered Jun 28 '20 at 17:43

1

'sizeof' returns the number of bytes that specified type takes. in this case sizeof(g->buffer) is equal to sizeof(int*) and because you are using x64 processor sizeof all pointers is 8.

answered Jun 28 '20 at 17:43

Ali Askari

515
3
8

Craig Estey · Accepted Answer · 2020-06-28T19:05:04.680

Your grid_new is allocating an array of grid structs and not a single grid with the correct number of elements.

You need to set buffer

Also, the number of elements in the grid is based on width/height and not sizeof(g->buffer) which is the size of the pointer and not the area to which it points

Here's the refactored code:

const int group_height = 4;
const int group_width = 2;

typedef struct {
    int width;
    int height;
    int *buffer;
} grid;

grid *
grid_new(int grid_width, int grid_height)
{
    if ((grid_width % 2 != 0) || (grid_height % 4 != 0))
        return NULL;

    grid *p_grid = calloc(1,sizeof(*p_grid));

    // FIXME -- why???
    grid_width /= group_width;
    grid_height /= group_height;

    p_grid->width = grid_width;
    p_grid->height = grid_height;

    p_grid->buffer = calloc(grid_width * grid_height,sizeof(int));

    return p_grid;
}

void
grid_free(grid *p_grid)
{
    free(p_grid->buffer);
    free(p_grid);
}

void
grid_clear(grid *g)
{
    // ToDo: Iterate over all elements in the buffer
    int elements = g->width * g->height;

    printf("Elements: %i", elements);
}

I see. That makes sense. Gotta get used to that - switching from C++ to C comes with some neat surprises ;) Also is this the recommended style guideline for C, having the return type in a separate line above the function signature? — Kyu96, Jun 28 '20 at 19:41
_Most_ style guides recommend it. But the linux _kernel_ style guide says it should be on the same line. I do it for several reasons: (1) More clear/cleaner [IMO, based on 40 yrs experience] (2) You can find the function _definition_ easily with a `regex` (e.g.): `vi \`grep -rl ^grid_new[(] .\`` (3) It helps keep the lines < 80 chars [which _is_ recommended] — Craig Estey, Jun 28 '20 at 19:51

Rewrite C++ class in C

2 Answers2