Generic enum to text lookup in C

Question

Can my code from this question be improved? I am thiking of validating inputs, portability issues, etc

Please note that we must declare our strings at compile time. We code embedded systems and are not allowed to malloc().

---

[Update]

I forgot to state that our enums are non-contiguous and have a wide range, which can make a difference.

---

Here's some test data:

// +=+=+=+=+=+=+=+=+=+=+=+=+=+=
typedef enum
{
    north,
    south,
    east,
    west
} E_directions;

struct direction_datum
{
    E_directions direction;
    char         direction_name[6];
};

struct direction_datum direction_data[] =
{
    {north, "north"},
    {south, "south"},
    {east,  "east"},
    {west,  "west"},
};

// +=+=+=+=+=+=+=+=+=+=+=+=+=+=
typedef enum
{
    hearts,
    spades,
    diamonds,
    clubs,
} E_suits;

struct suit_datum
{
    E_suits suit;
    char    suit_name[9];
};

struct suit_datum suit_data[] =
{
    {hearts,   "hearts"},
    {spades,   "spades"},
    {diamonds, "diamonds",},
    {clubs,    "clubs"},
};

and, here's the code, which should get text from an enum, as defined by a struct of enum/string, where different structs can have different length arrays.:

#define ARRAY_LENGTH(A) sizeof(A) / sizeof(*A) 

char *Get_text_from_enum(int enum_value, 
                         void *array, 
                         unsigned int array_length, 
                         unsigned int size_of_array_entry)
{
    unsigned int i;
    unsigned int offset;

    for (i = 0; i < array_length; i++)
    {
        offset = i * size_of_array_entry;

        if ((int) * ((int *) (array+ offset)) == enum_value)
            return (char *)  (array + offset + sizeof(int));
    }

    return NULL;
}


printf("Expect south,    got %s\n", 
          Get_text_from_enum(south,    
                             direction_data, 
                             ARRAY_LENGTH(direction_data),  
                             sizeof(direction_data[0])));

printf("Expect diamonds, got %s\n", 
          Get_text_from_enum(diamonds, 
                             suit_data,      
                             ARRAY_LENGTH(suit_data),       
                             sizeof(suit_data[0])));

Answer 1

Illegal void pointer arithmetic

This code is illegal in C:

void *array;
... (array + offset)

You are not allowed to do pointer arithmetic on a void * pointer. You should use char * instead of void *.

Simplifications

I would suggest defining a generic structure like this:

typedef struct EnumName {
    int         value;
    const char *name;
} EnumName;

to avoid all of the struct length shenanigans. You can also make it a rule that each EnumName array must be terminated with a NULL name. This rule allows you to avoid passing in an array length argument. Then your code would look like this:

const EnumName direction_data[] =
{
    {north, "north"},
    {south, "south"},
    {east,  "east" },
    {west,  "west" },
    {0,      NULL  },
};

const char *Get_text_from_enum(int enum_value, const EnumName *names)
{
    while (names->value != enum_value && names->name != NULL)
        names++;
    return names->name;
}

int main(void)
{
    printf("Expect south,    got %s\n",
            Get_text_from_enum(south, direction_data));
}

Answer 2

Pointer issues:

• You should avoid void* where it isn't necessary. In this case, there is no reason why you should use it, use a char* instead. Pointer arithmetic on void* in not even valid C, this is a non-standard GCC extension.
• You should use const-correctness for the pointers where the contents aren't modified.
• (int) * ((int *)something) The cast to int is superfluous clutter. The contents pointed at by an int* is always type int. This could as well be written as *(int*)something.
• In case your function returns a NULL pointer, your program will crash since you never check for this.

Functionality:

• The actual enumerations should be improved in the ways I described here at SO. This would make your whole look-up function pointless. Instead you would be able to directly access the string through an array index, which is the same as the enumeration constant. This improves readability and execution speed both.
• printf and other functions from stdio.h should be avoided in embedded systems production code, since they are very inefficient. But also bug-prone, because of the non-existent type safety.
• Never use undocumented "magic numbers" such as 9 and 6 in your code. Always use pre-defined constants (#defines), so that the code turns self-documenting.

Type issues:

• The native so-called "primitive data types" in c (int, long etc) should never be used in embedded systems. You should always use stdint.h types to avoid implicit type promotion bugs, wrong signedness and portability issues.
• Never used signed integers as the default type. Always use unsigned integers unless you actually need negative numbers. This is particularly important in embedded systems, where bit-wise operations are common. Mixing bit-wise operations and signed numbers opens up a whole bag of worms of possible severe bugs that could be avoided just by using unsigned types.
• For maximum portability, array lengths should be expressed with the type size_t and not unsigned int.
• int enum_value should be E_directions enum_value.

Overall, you will find most of the above remarks in the MISRA-C coding standard. I think you would benefit from reading that document and possibly also consider writing MISRA compliant programs in the future, to reduce bugs.