Good Form: Pointer vs. local variable vs. Array Index

Question

Forgive me if this comes across as a trivial question - I'm usually a control systems guy (plc's and automation) but have found myself involved in some embedded micro-controller and PC projects lately.

Let's say I have a function that accepts a pointer to an array of 'command bytes', typically 5 or 10 bytes in length, like so:

char cmd_i2c_read(unsigned char *cmd, unsigned short cmd_len) { ... }

I want to decode the command bytes (*cmd).

Is it better form to:

Create local variables indicating the purpose of each byte:

unsigned char device_address = cmd[2];
unsigned char register_address = cmd[3];
unsigned char num_bytes = cmd[4];

// use the local variables:
if(num_bytes &le 0xFF) {
    do_stuff(device_address, register_address, num_bytes);
}

Create local pointers:

unsigned char *device_address = &cmd[2];
unsigned char *register_address = &cmd[3];
unsigned char *num_bytes = &cmd[4];

// use the pointers:
if(*num_bytes &le 0xFF) {
    do_stuff(*device_address, *register_address, *num_bytes);
}

Index the *cmd array directly:
if(cmd[4] <= 0xFF) { do_stuff(cmd[2], cmd[3], cmd[4]); }

And then we can start arguing about if the if statement should be on a single line or if 0xFF should be made into a constant. :-) — Kevin Gale
– Kevin Gale, Commented Feb 23, 2010 at 21:10

John Knoeller · Accepted Answer · 2010-02-23 22:52:11Z

4

Option 1 is clear but a bit wordy. I don't like 2 at all, and 3 is hard to understand. Personally I prefer to use structs for this sort of thing.

typedef struct  {
   unsigned char whatever[2];
   unsigned char device_address;
   unsigned char register_address;
   unsigned char num_bytes;
   }  CMD;

CMD * pcmd = (CMD *)&cmd[0];

// use the local variables:
if(num_bytes ≤ 0xFF) {
    do_stuff(pcmd->device_address, pcmd->register_address, pcmd->num_bytes);

edited Feb 23, 2010 at 22:52

answered Feb 23, 2010 at 20:57

John Knoeller

34.3k4 gold badges66 silver badges93 bronze badges

Sign up to request clarification or add additional context in comments.

7 Comments

Alok Singhal Over a year ago

Doesn't this assume that CMD can be arbitrarily-aligned?

Tim Schaeffer Over a year ago

Obligatory struct padding warning: This, of course, should be done with care. The struct is not guaranteed to have its elements contiguous in memory, so the struct may not match exactly the layout of the input data. If your compiler has a something like gcc's __attribute__((packed)), it would at least be a good idea to use that. This (as it is) may not even port bw differing versions of the same compiler.

John Knoeller Over a year ago

@Alok: as long as the members of CMD are byte sized, there are no alignment issues.

bta Over a year ago

Using a struct overlay like this avoids the data duplication of #1, the extra variables of #1 and #2, and the ambiguity of #3. As an added benefit, it reinforces the idea that these values are related to each other.

Alok Singhal Over a year ago

@John: I guess my question was if that is true according to the standard.

|

JonH · Accepted Answer · 2010-02-23 20:50:22Z

3

I prefer item 3 but it comes down to preference.

answered Feb 23, 2010 at 20:50

JonH

33.2k13 gold badges94 silver badges149 bronze badges

Comments

Wookai · Accepted Answer · 2010-02-23 20:57:24Z

3

IMHO, the first way is better. It is much easier to read than number 3, because you don't need to know the signature of the function to understand what the parameters are.

For bigger data structures, I would go with number 2, i.e. use pointer such that you don't have to copy the values. But in this case, the difference is not significant, and I think that the */& slightly reduce the readability.

edited Feb 23, 2010 at 20:57

answered Feb 23, 2010 at 20:51

Wookai

21.9k16 gold badges76 silver badges86 bronze badges

7 Comments

Kevin Gale Over a year ago

+1 I agree it's a lot easier to read device_address than cmd[2].

Javier Over a year ago

#3 would be the easiest to read if instead of using numbers, there were predefined constants for the indexes.

David Thornley Over a year ago

Why would 2 do better than 1 at not losing space? In 1, three unsigned chars are created, and in 2 three unsigned char *s are created.

JonH Over a year ago

@Kevin - but that is only because you know cmd[2] is a device address, so what is the difference? I could of easily just assigned it to a temp variable called device_address to fulfill the wish...My point is the programmer knows that cmd[2] or cmd[index] represents a specific value for a specific type. Otherwise he / she wouldn't know how to call do_stuff() //in this case...

Wookai Over a year ago

@David: I edited my answer ;) ! I was referring to bigger data structures, where a pointer is really smaller than the whole data. But in this case, it does not change much !

|

Max Shawabkeh · Accepted Answer · 2010-02-23 20:53:01Z

2

Definitely #1. It makes the rest of the code much more readable, and using a pointer where a normal variable will do complicates the matter for no reason. The compiler will optimize away whatever microscopic performance gains you might get from #3.

answered Feb 23, 2010 at 20:53

Max Shawabkeh

38.7k10 gold badges85 silver badges92 bronze badges

2 Comments

Javier Over a year ago

with a dumb compiler, #3 would be the slowest in most cases.

caf Over a year ago

You could also make the local variables const - you should find that with optimisation enabled, the compiler will optimise those variables out of existence.

Andrew O'Reilly · Accepted Answer · 2010-02-23 22:30:59Z

1

If it were me I'd write this as #3 but with symbolic names for the array indices to improve readability:-

#define DEVICE_ADDRESS 2
#define REGISTER_ADDRESS 3
#define NUM_BYTES 4

if (cmd[NUM_BYTES] <= 0xFF) {
    do_stuff(cmd[DEVICE_ADDRESS], cmd[REGISTER_ADDRESS], cmd[NUM_BYTES]);
}

You can of course replace the macros with const ints, enums, etc. The reason why I like this option is that the other two options require the use of additional local variables. The compiler may or may not optimize these away depending on its implementation and your chosen level of optimization but they just seem like an unnecessary level of extra indirection to me.

edited Feb 23, 2010 at 22:30

answered Feb 23, 2010 at 22:11

Andrew O'Reilly

1,65313 silver badges15 bronze badges

Comments

Thomas Schaub · Accepted Answer · 2010-02-23 21:11:33Z

0

I like 1 the most, it's much more readable than the others.

If performance is an important issue (i mean important like in "we need every 0.01% speedup we can get"), you will have to benchmark cause it really depends on the compiler which sequence will end up in the fastest code (1 may have unnecessary copys, 2 may contain superflous loads due to pointer aliasing restrictions, 3 may lead to superflous loads if the register allocator is really messed up)

answered Feb 23, 2010 at 21:11

Thomas Schaub

4183 silver badges11 bronze badges

Comments

Liz Albin · Accepted Answer · 2010-02-23 22:31:20Z

0

To me it depends on What You'll Be Doing with your buffer,

All things being equal though, I'd probably prefer to have the buffer be a struct (assume alignment warings) and, failing that, to directly index the buffer, although not with magic numbers.

answered Feb 23, 2010 at 22:31

Liz Albin

1,4998 silver badges8 bronze badges

Collectives™ on Stack Overflow

Good Form: Pointer vs. local variable vs. Array Index

7 Answers 7

7 Comments

Comments

7 Comments

2 Comments

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

7 Answers 7

7 Comments

Comments

7 Comments

2 Comments

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related