1

I'm inexperienced with C, and working on a microcontroller with messages stored in arrays where each byte does something different. How do I give each element of the array a human-readable name instead of referencing them as msg[1], msg[2], etc.?

Is this what structs are for? But "you cannot make assumptions about the binary layout of a structure, as it may have padding between fields."

Should I just use macros like this? (I know "macros are bad", but the code is already full of them)

#define MSG_ID       msg[0]
#define MSG_COMMAND  msg[1]

Oh! Or I guess I could just do

MSG_ID = 0;
MSG_COMMAND = 1;
MSG[MSG_ID];

That's probably better, if a little uglier.

endolith
  • 25,479
  • 34
  • 128
  • 192
  • Why do you think "macros are bad"? – glglgl Jan 25 '12 at 15:59
  • 1
    I would do enum { MSG_ID, MSG_COMMAND }; and use the enum to index the array. – lvella Jan 25 '12 at 16:02
  • @Ben: How? An enum is just like my second example, no? And can you count on the values of the enum to be sequential? Two different systems have to agree on the order of bytes. – endolith Jan 25 '12 at 16:02
  • @glglgl: Because that's what I've read, and I've already seen that working with them is a lot more error-prone? – endolith Jan 25 '12 at 16:08
  • 1
    _Function-like macros_ are bad. There are no problems with using macros for constants, or as in this case to improve readability. I would use those two macros you have listed, or even better, do as lvella suggests and name the _indices_ rather than the actual array items. – Lundin Jan 25 '12 at 16:08
  • oh, yes an enum would be for your second example. I am not sure if it is guaranteed to be sequential however you can assign specific values eg. enum message_type `{MSG_ID = 1, MSG_COMMAND = 2, ... };` – Ben Jan 25 '12 at 16:12
  • 1
    @Ben Unless you initialize the enum explicitly as in your example, it is guaranteed to be enumerated as 0, 1, 2 etc corresponding to array indexing. – Lundin Jan 25 '12 at 16:14

3 Answers3

3

If you want to go that route, use a macro, for sure, but make them better than what you suggest:

#define MSG_ID(x)      (x)[0]
#define MSG_COMMAND(x) (x)[1]

Which will allow the code to name the arrays in ways that make sense, instead of ways that work with the macro.

Otherwise, you can define constants for the indexes instead (sorry I could not come up with better names for them...):

#define IDX_MSG_ID      0
#define IDX_MSG_COMMAND 1

And macros are not bad if they are used responsibly. This kind of "simple aliasing" is one of the cases where macros help making the code easier to read and understand, provided the macros are named appropriately and well documented.

Edit: per @Lundin's comments, the best way to improve readability and safety of the code is to introduce a type and a set of functions, like so (assuming you store in char and a message is MESSAGE_SIZE long):

typedef char MESSAGE[MESSAGE_SIZE];
char get_message_id(MESSAGE msg) { return msg[0]; }
char get_message_command(MESSAGE msg) { return msg[1]; }

This method, though it brings some level of type safety and allows you to abstract the storage away from the use, also introduces call overhead, which in microcontroller world might be problematic. The compiler may alleviate some of this through inlining the functions (which you could incentize by adding the inline keyword to the definitions).

Romain
  • 12,679
  • 3
  • 41
  • 54
  • Oh, I'm only working with one message at a time. Assembling it, shipping it out, and then starting over again with a new message in the same variable. – endolith Jan 25 '12 at 16:00
  • 1
    Even then, it's better to allow the code that uses the macros to name their variables freely. Don't use stuff in a macro if the macro didn't declare it, unless it's "globally available" (which is usually a sign of poor design decisions). – Romain Jan 25 '12 at 16:01
  • 1
    How exactly is this making the code more readable? I think it does the opposite. `MSG_ID(msg)` will almost certainly confuse the reader, while `msg[MSG_ID]` will not. Also, you have no safety whatsoever in these macros, you can show anything inside them. – Lundin Jan 25 '12 at 16:10
  • 1
    @Lundin I agree on the principle, using index aliasing is better than the plain macro. Mostly because of the type safety issue. Since it's microcontroller stuff, I assumed one wants to spare memory/call overhead. – Romain Jan 25 '12 at 16:13
  • Call overhead? There is no overhead in any of the mentioned ways. – Lundin Jan 25 '12 at 16:16
  • I think I'm going to use the macros as index numbers idea. – endolith Jan 25 '12 at 16:39
  • 1
    @Lundin Using a typedef + functions brings type safety and semantics, also call overhead, memory consumption. I didn't mention these because of the said overhead. – Romain Jan 25 '12 at 16:41
  • @Romain: You could mention them with a note that it might not be appropriate for microcontrollers – endolith Jan 25 '12 at 19:52
2

The most natural concept for naming a set of integers in C are enumerations:

enum msg_pos { msg_id, msg_command, };

By default they start counting at 0 and increment by one. You would then access a field by msg[msg_id] for example.

Jens Gustedt
  • 76,821
  • 6
  • 102
  • 177
  • 1
    I'm no big fan of using `enum`s this way. It feels dirty, I don't know why... I still +1, because despite it looking dirty to me, it's a very valid way of doing things... – Romain Jan 30 '12 at 09:05
1

It's fine to use a struct if you take the time to figure out how your compiler lays them out, and structs can very useful in embedded programming. It will always lay out the members in order, but there may be padding if you are not on an 8-bit micro. GCC has a "packed" attribute you can apply to the struct to prohibit padding, and some other compilers have a similar feature.

David Grayson
  • 84,103
  • 24
  • 152
  • 189
  • 2
    The problem is that the packing method (#pragma pack or whatever) isn't standardized. So to be picky, structs are non-portable. – Lundin Jan 25 '12 at 16:12
  • Only a perverse implementation would pad members of a struct differently than similar members of an array. – Bo Persson Jan 25 '12 at 17:29