4

I was looking at some C++ networking code. When I went to find out how large the buffer was, I found something like this:

static const uint32_t MAX_COMMAND_BUFFER =
        sizeof(struct S1) | sizeof(struct S2) | sizeof(struct S3) |
        sizeof(struct S4) | sizeof(struct S5) | sizeof(struct S6);

I believe the basic idea here is that the buffer can contain any one of these 6 structs. This is declared at an outer scope, so its available for things like a later outer-scope uint8_t array size.

Now normally here I'd expect to see something using either the std::max() function, or the tertinary operator to calculate the exact size of the largest of those 6 structs. I've never seen the above idiom before, and had to stop to even convince myself it would work.

So since I'm new at this, I'd like in general to know how valid this is, and what are the benefits and drawbacks of doing it.

What I can see is -

pro:

  • It should always give you a size at least as large as your largest struct size. Which is all you really need if space isn't at a premium.
  • It appears to be calculable at compile time.
  • It should work even with very old C++ (or even C) compilers.

con:

  • It may make the size nearly twice as large as necessary.
  • It makes it much more difficult for a human to figure out offline exactly how large the buffer is.
  • Its weird / unexpected.
T.E.D.
  • 44,016
  • 10
  • 73
  • 134
  • 1
    Depending on what compiler the code was written for, it may have been an optimization, especially since the compiler (as you said) can calculate the whole thing at compile-time. – kirbyfan64sos Jun 27 '16 at 20:50

4 Answers4

3

It's valid in the sense that MAX_COMMAND_BUFFER will always be at least as large as the maximum of all the sizes of those structs. It's easy to verify this - bitwise |-ing all the sizes together will give you a value that has all the same bits on as the largest size, plus possibly some others.

However, it's super confusing and you have to stop and think about it. Plus, if you have two sizes that are 8 and 7 you'll end up with 15, which is probably not what you want to happen. Thankfully, std::max has a constexpr overload that takes an initializer_list<T>, so just use that:

static constexpr size_t MAX_COMMAND_BUFFER =
//               ^^^^^^
        std::max({sizeof(struct S1), sizeof(struct S2), ..., sizeof(struct S6)});

If your compiler doesn't support that overload, it's pretty straightforward to write a variadic function template to accomplish the same thing.

Barry
  • 286,269
  • 29
  • 621
  • 977
  • Could you put a C++ standard version on that alternative? We have to support some older (eg: C++ 11) compiler versions, and I suspect in the general case we aren't the only ones. – T.E.D. Jun 27 '16 at 20:59
2

Just write down the intention:

#include <algorithm>
struct S1 {};
struct S2 {};
struct S3 {};
int main() {
    static const unsigned maximum = std::max({sizeof(S1), sizeof(S2), sizeof(S3) /*, ... */});
}

There is no 'pro' in obfuscating things.

  • Definitely agree with the last sentence, and I do appreciate ideas for "fixing" this (since I may do that). However, I'm particularly interested in if this is some well-known idiom that I've somehow missed my entire 31 year career, and if not, do I have a good handle on the pros/cons so I can intelligently decide (and back up my decision). – T.E.D. Jun 27 '16 at 21:03
2

I agree with you, I think the way they did it is funky and suboptimal.

The way I used to do this sort of thing was to make a union of all the structs, and then use the size of the union...

union Commands {
  S1 s1;
  S2 s2;
  ...
  S6 s6;
};

sizeof(Commands);

And if you combine this with a struct holding your packet header, followed by the union...

struct Packet {
  Header header;
  Commands command;
};

You can set one pointer at your buffer, and have easy access to all your data...

Packet *packet = (Packet)&buf[0];
switch(packet->header.command_type) {
  case COMMAND_DO_STUFF:
    printf("%d\n", packet->command.s1.stuff);
    break;
  case COMMAND_QUIT:
    ..
    break;
};
Dav3xor
  • 386
  • 2
  • 6
0

That looks pretty hokey. I'd use sizeof(union { S1 s1; S2 s2; S3 s3; S4 s4; S5 s5; S6 s6; }). (not tested)

Pete Becker
  • 74,985
  • 8
  • 76
  • 165