gberth
gberth

Reputation: 744

C - Populate a generic struct inside a function without malloc

I'm trying to build a generic function that can populate a struct without any dynamic memory allocation.

The following code is a naive example of what I'm trying to do. This code will not compile as incomplete type 'void' is not assignable.

Please note that this is a toy example to highlight my problems. I don't really want to convert colours; I just want to highlight that the structures will be different in data types and size.

#include <stdio.h>

typedef struct {
    int r;
    int g;
    int b;
} rgb_t;

typedef struct {
    float c;
    float m;
    float y;
    float k;
} cmyk_t;

typedef enum { RGB, CMYK } color_t;

void convert_hex_to_color(long hex, color_t colorType, void* const out) {
    if (colorType == RGB) {
        rgb_t temp = { 0 };
        // Insert some conversion math here....
        temp.r = 1;
        temp.g = 2;
        temp.b = 3;
        *out = temp; //< [!]
    } else
    if (colorType == CMYK) {
        cmyk_t temp = { 0 };
        // Insert some conversion math here....
        temp.c = 1.0;
        temp.m = 2.0;
        temp.y = 3.0;
        temp.k = 4.0;
        *out = temp; //< [!]
    }
}

int main(void) {
    // Given
    long hex = 348576;
    rgb_t mydata = { 0 };
    convert_hex_to_color(hex, RGB, (void*)(&mydata));

    // Then
    printf("RGB = %i,%i,%i\r\n", mydata.r, mydata.g, mydata.b);
    return 0;
}

For some additional context, I'm using C11 on an embedded system target.

What is the best[1] way to do this? Macro? Union?

Regards,
Gabriel

[1] I would define "best" as a good compromise between readability and safety.

Upvotes: 3

Views: 456

Answers (4)

Christian Gibbons
Christian Gibbons

Reputation: 4370

Frame challenge: It seems like you want to perform a different operation depending on the type that you pass into this function. Instead of using an enum to tell it what type you're passing in, and branching based on that enum, make use of C11's _Generic to handle that, and you don't even need to explicitly tell it what the type is on each call:

#include <stdio.h>

typedef struct {
    int r;
    int g;
    int b;
} rgb_t;

typedef struct {
    float c;
    float m;
    float y;
    float k;
} cmyk_t;

inline void convert_hex_to_color_rgb(long hex, rgb_t *const out) {
    (void) hex; // or whatever you're planning to do with 'hex'
    out->r = 1;
    out->g = 2;
    out->b = 3;
}

inline void convert_hex_to_color_cmyk(long hex, cmyk_t *const out) {
    (void) hex; // or whatever you're planning to do with 'hex'
    out->c = 1.0;
    out->m = 2.0;
    out->y = 3.0;
    out->k = 4.0;
}

#define convert_hex_to_color(hex, out) _Generic((out), \
        rgb_t *: convert_hex_to_color_rgb((hex), (rgb_t *)(out)), \
        cmyk_t *: convert_hex_to_color_cmyk((hex), (cmyk_t *)(out)) \
)

int main(void) {
    // Given
    long hex = 348576;
    rgb_t mydata = { 0 };
    cmyk_t mydatac = { 0 };
    convert_hex_to_color(hex, &mydata);
    convert_hex_to_color(hex, &mydatac);

    // Then
    printf("RGB = %i,%i,%i\r\n", mydata.r, mydata.g, mydata.b);
    printf("CMYK = %f,%f,%f,%f\r\n", mydatac.c, mydatac.m, mydatac.y, mydatac.k);
    return 0;
}

Upvotes: 3

Lundin
Lundin

Reputation: 214060

The "best" way is not to mix unrelated structures in the same function, or in the same memory area for that matter. That's just messy design.

If you need to keep a unison API for two different forms of data, then a typesafe function-like macro might be one idea. You can fake such a macro to have a syntax similar to passing the data by pointer

void convert_hex_to_color(long hex, type* data)

But then use C11 _Generic to actually determine the correct type to use, rather than using dangerous void pointers. Since you can't pass parameters "by reference" to macros, you'd have to sneak in a variable assignment in there. Example:

#include <stdio.h>

typedef struct {
    int r;
    int g;
    int b;
} rgb_t;

typedef struct {
    float c;
    float m;
    float y;
    float k;
} cmyk_t;



void convert_hex_to_color(long hex, void* data);
  /* 
     Pretty prototype just for code documentation purposes. 
     Never actually defined or called - the actual macro will "mock" this function. 
  */

#define convert_hex_to_color(hex, output) ( *(output) = _Generic(*(output), \
  rgb_t:  (rgb_t){ .r=1, .g=2, .b=3 }, \
  cmyk_t: (cmyk_t){ .c=1.0, .m=2.0, .y=3.0, .k=4.0 } ) )


int main(void) {
    // Given
    long hex = 348576;
    rgb_t  myrgb  = { 0 };
    cmyk_t mycmyk = { 0 };

    convert_hex_to_color(hex, &myrgb);
    convert_hex_to_color(hex, &mycmyk);

    printf("RGB  = %i,%i,%i\r\n", myrgb.r, myrgb.g, myrgb.b);
    printf("CMYK = %f,%f,%f,%f\r\n", mycmyk.c, mycmyk.m, mycmyk.y, mycmyk.k);
    return 0;
}

Output:

RGB  = 1,2,3
CMYK = 1.000000,2.000000,3.000000,4.000000

Just be aware that the _Generic support for type qualifiers (const etc) was shaky in C11 - some C11 compilers treated const rgb_t different from rgb_t, others treated them the same. This was one of the "bug fixes" in C17, so use C17 if available.

Upvotes: 4

chqrlie
chqrlie

Reputation: 144820

The reason for the error is it is invalid to store via a void pointer: the compiler does not know what to store. You could cast the pointer as *(rgb_t *)out = temp; or *(cmyk_t *)out = temp;

Alternately, you could define temp as a pointer to the appropriate structure type and initialize it directly from out, without the cast that is not needed in C:

void convert_hex_to_color(long hex, color_t colorType, void *out) {
    if (colorType == RGB) {
        rgb_t *temp = out;
        // Insert some conversion math here....
        temp->r = 1;
        temp->g = 2;
        temp->b = 3;
    } else
    if (colorType == CMYK) {
        cmyk_t *temp = out;
        // Insert some conversion math here....
        temp->c = 1.0;
        temp->m = 2.0;
        temp->y = 3.0;
        temp->k = 4.0;
    }
}

Note that the cast is not needed in C:

int main(void) {
    // Given
    long hex = 348576;
    rgb_t mydata = { 0 };
    convert_hex_to_color(hex, RGB, &mydata);

    // Then
    printf("RGB = %i,%i,%i\r\n", mydata.r, mydata.g, mydata.b);
    return 0;
}

Upvotes: 8

Blindy
Blindy

Reputation: 67386

rgb_t temp = {0};

So that declares a variable on the stack of type rgb_t. So far so good, though you don't need that 0.

*out = temp;

Here is your problem: in C you can only copy memory of the same type. Ever. This has nothing to do with malloc, as your title suggests, this is just the basic language specification. Sure, some types provide implicit casts, but void* is not one of them.

So if you're copying a structure (rgb_t on the right side), the destination has to be of the same type. So change the line to this:

*(rgb_t *)out = temp;

Upvotes: 4

Related Questions