c - comparing two structure

Thread Starter

bug13

Joined Feb 13, 2012
2,002
Nothing to stop you grouping them in a struct. You could use 32 bit ints, then only need 3, but that may have a performance hit on an 8 bit processor. A general idea is that lots of repeated simple clear code is easier to understand, debug and maintain than a small amount of obscure clever code. In the early days of C, programmers seemed to delight in getting as much done with the least possible source code by using obscure oh-so-clever features of the language (look at some of the standard library implementation for examples of that), but thinking has changed, and KISS is the motto now.
So you mean this:
Code:
typedef struct{
  uint32_t settings1;
  uint32_t settings2;
  uint32_t settings3;
}__SETTINGS__;

__SETTINGS__ settings;

checkBit(settings.settings1, BIT_MASK)
 

xox

Joined Sep 8, 2017
838
Yes. It always works. Every compiler, every processor, every data type. memcmp can fail for floats as an example (even though this thread is not about floats) because some identical float values have multiple bit representations.

The float comparison problem is an excellent example of why it is risky.
[...]
This may sound far fetched and a long way away from a question about a student project, but I've been there, and getting into the habit of writing safe clear maintainable reliable code can never start too soon.
That's...insane. We aren't even talking about float comparison here. The memcmp function compares things byte for byte. It's fast (usually compiles down to repne/scas instructions) and guaranteed not to throw a segmentation fault even when comparing the bits/bytes of a float. And way less bug prone then member by member.
 

miniwinwm

Joined Feb 2, 2018
68
That's...insane. We aren't even talking about float comparison here. The memcmp function compares things byte for byte. It's fast (usually compiles down to repne/scas instructions) and guaranteed not to throw a segmentation fault even when comparing the bits/bytes of a float. And way less bug prone then member by member.
I know we're not talking about floats, which is why I said we're not talking about floats, but it's an example of where things can go badly wrong. As I said, we'll have to agree to differ. We have opinions formed from our experiences, and our experiences are doubtless different, so our opinions are different. As I also said, the thinking now is repeated simple easy to understand code, even if there's a lot of it, is more reliable, safer and easier to maintain than 1 line that does it all now, but might not in 2 decades time when the code is modified (or even now when the code is developed by a large multi-national team of widely different capabilities). That is the time when dangerous bugs are most likely to enter a codebase. How easy would it be for a code maintainer to add a new variable to a struct years down the line and not notice that the alignment is wrong, or takes a single bit out of a C bitfield as in the OP's original example, and the memcmp then fails, maybe only very occasionally, and not during testing? Very easy. This kind of bug happens all the time. And that's the reason why things like comparing structs with a memcmp are not allowed in commercial quality safety critical code. You might think it's insane, but industry that produces code that people's safety depends on does not. The reason is - it always works. Reassuring to know next time you fly, press the throttle on your car or are wired up to a device that beeps when in hospital.
 
Last edited:

miniwinwm

Joined Feb 2, 2018
68
So you mean this:
Code:
typedef struct{
  uint32_t settings1;
  uint32_t settings2;
  uint32_t settings3;
}__SETTINGS__;

__SETTINGS__ settings;

checkBit(settings.settings1, BIT_MASK)
I don't want to overegg the weaknesses of C bifields, which I probably have done in this thread. If you are familiar with them and they do the job, then use them. Just be aware they do have some weaknesses, so keep that in the back of your mind for the future. For your task though, they will be fine.

If you do use your own bitfields, don't forget that it's optimal to use the word size of a processor when you can, which for a PIC is 8 bits. If you use 32 bits (I know I suggested it) there will be lots of unnecessary loading, shifting and storing of bytes the processor will need to do.

A final trivial style thing which doesn't affect your code at all but is good to know - in C identifiers beginning with _ are usually reserved for system things, like compiler #defines and internal identifiers in your OS (if you have one) or the standard library. They are not normally used for user code. A common way to end typedef's is with _t, as in uint8_t etc.
 

Thread Starter

bug13

Joined Feb 13, 2012
2,002
I don't want to overegg the weaknesses of C bifields, which I probably have done in this thread. If you are familiar with them and they do the job, then use them. Just be aware they do have some weaknesses, so keep that in the back of your mind for the future. For your task though, they will be fine.

If you do use your own bitfields, don't forget that it's optimal to use the word size of a processor when you can, which for a PIC is 8 bits. If you use 32 bits (I know I suggested it) there will be lots of unnecessary loading, shifting and storing of bytes the processor will need to do.

A final trivial style thing which doesn't affect your code at all but is good to know - in C identifiers beginning with _ are usually reserved for system things, like compiler #defines and internal identifiers in your OS (if you have one) or the standard library. They are not normally used for user code. A common way to end typedef's is with _t, as in uint8_t etc.
Cool, will keep them in mind, thanks!
 

Thread Starter

bug13

Joined Feb 13, 2012
2,002
So compares member by member is the way to go.

That leads to anther question, so if I have a struct like this:
Code:
typedef struct{
  m1 : 1;
  m2 : 1;
}foo_t;
And my compareStruct() is like this:
Code:
//compare strut member by member
uint8_t compareStruct(foo_t s1, foo_t s2){
  if (s1.m1 != s2.m1) return 0;
  etc...
  return 1;
}
What if I add a new member to my struct like this:
Code:
typedef struct{
  m1 : 1;
  m2 : 1;
  m_new : 1;
}foo_t;
Is there a way to put something in my compare function to detect the struct is changed? So I can stop the compiler or at least print out a warning???

Because if I add a new member in my structure, my compare function won't know how to compare the newly added member in the structure, I can see trouble from here down the track.
 

bogosort

Joined Sep 24, 2011
696
Is there a way to put something in my compare function to detect the struct is changed?
Note that you don't need to pass the entirety of each struct to your compare function; passing pointers to each struct will save a bunch of needless stack ops.

As for your question, I wouldn't recommend this for production code, but it's relatively straightforward to iterate over a struct if its members are all of the same type:

Code:
typedef struct
{
    int m1;
    int m2;
    int m3;
} foo_t;

// assumes all members of foo_t are int!
int compare_foo(const foo_t *s, const foo_t *t)
{
    int i;
    int num_members = sizeof *s / sizeof(int);

    for ( i = 0; i < num_members; i++ )
    {
        int fooA = *(((int *)s) + i);
        int fooB = *(((int *)t) + i);

        if ( fooA != fooB )
            return 0;
    }

    return 1;
}


int main(void)
{
    foo_t s1, s2;

    s1.m1 = 10;
    s1.m2 = 20;
    s1.m3 = 30;

    s2.m1 = 10;
    s2.m2 = 25;
    s2.m3 = 30;

    if ( !compare_foo(&s1, &s2) )
        puts( "structs are different" );
    else
        puts( "structs are the same" );

    return 0;
}
 

Thread Starter

bug13

Joined Feb 13, 2012
2,002
Note that you don't need to pass the entirety of each struct to your compare function; passing pointers to each struct will save a bunch of needless stack ops.

As for your question, I wouldn't recommend this for production code, but it's relatively straightforward to iterate over a struct if its members are all of the same type:

Code:
typedef struct
{
    int m1;
    int m2;
    int m3;
} foo_t;

// assumes all members of foo_t are int!
int compare_foo(const foo_t *s, const foo_t *t)
{
    int i;
    int num_members = sizeof *s / sizeof(int);

    for ( i = 0; i < num_members; i++ )
    {
        int fooA = *(((int *)s) + i);
        int fooB = *(((int *)t) + i);

        if ( fooA != fooB )
            return 0;
    }

    return 1;
}


int main(void)
{
    foo_t s1, s2;

    s1.m1 = 10;
    s1.m2 = 20;
    s1.m3 = 30;

    s2.m1 = 10;
    s2.m2 = 25;
    s2.m3 = 30;

    if ( !compare_foo(&s1, &s2) )
        puts( "structs are different" );
    else
        puts( "structs are the same" );

    return 0;
}
I thought we have already decided this is not a good idea??
 

WBahn

Joined Mar 31, 2012
30,088
Note that you don't need to pass the entirety of each struct to your compare function; passing pointers to each struct will save a bunch of needless stack ops.

As for your question, I wouldn't recommend this for production code, but it's relatively straightforward to iterate over a struct if its members are all of the same type:

Code:
typedef struct
{
    int m1;
    int m2;
    int m3;
} foo_t;

// assumes all members of foo_t are int!
int compare_foo(const foo_t *s, const foo_t *t)
{
    int i;
    int num_members = sizeof *s / sizeof(int);

    for ( i = 0; i < num_members; i++ )
    {
        int fooA = *(((int *)s) + i);
        int fooB = *(((int *)t) + i);

        if ( fooA != fooB )
            return 0;
    }

    return 1;
}


int main(void)
{
    foo_t s1, s2;

    s1.m1 = 10;
    s1.m2 = 20;
    s1.m3 = 30;

    s2.m1 = 10;
    s2.m2 = 25;
    s2.m3 = 30;

    if ( !compare_foo(&s1, &s2) )
        puts( "structs are different" );
    else
        puts( "structs are the same" );

    return 0;
}
A problem that this code can have is that it assumes there are no padding bytes in there. What if the compiler pads the structure elements to, say, 8 byte boundaries and your data type is 4 bit each. You are essentially doing a sloppy memcmp operation.
 

miniwinwm

Joined Feb 2, 2018
68
I thought we have already decided this is not a good idea??
It depends what you are doing. A student project, a hobbyist at home, or maybe even a small commercial project where there are no safety consequences, no code re-use, an individual or a small team and a short-lived product, then you'll get away with it. Probably. However, a large, complex, industrial, safety-critical, long-term project in a regulated environment with formal reviews, coding standards conformance checks, automated static code analysis - then no, because it will be picked up and failed by one of those 3 checks.
 

WBahn

Joined Mar 31, 2012
30,088
I haven't read every post with a fine tooth comb, but one thing that strikes me as missing in the conversation is a clear definition of what it even means for two structures to be "equal" in the first place.

There are three reasonable interpretations that come to mind and they are all different.

1) They are the SAME structure, as indicated by the address at which they are stored being the same.

This type of equality can be checked for by comparing the pointers to the two structures in question.

Conceivably I could define two structures as part of a union in which they could be at the same address and not be the same structure, but if I'm playing those kinds of games (and there CAN be good reasons for doing something like that) then the onus is on me to get the details right which means I'm not looking for some general way of doing things, but rather I need to have my act together and know what the hell I am doing at a very low level and take responsibility for getting it right.

2) They are bit-for-bit identical copies of each other.

This type of equality can be checked using memcmp().

3) The two structures contain the same information.

This is the tricky one.

For instance, let's say that I have a floating point value named flt in a structure and in one structure I set

a.flt = 1.0;

and in the other I set

b.flt = a.flt / 10.0 * 10.0;

All other structure elements are bitwise equal.

Do I want or need structures 'a' and 'b' to evaluate as being equal to each other?

If I do, then since they almost certainly are NOT bitwise equal, I need to check for member-wise equality and, in doing so, properly determine if two floating point values are sufficiently close as to be considered "equal".
 

Thread Starter

bug13

Joined Feb 13, 2012
2,002
I haven't read every post with a fine tooth comb, but one thing that strikes me as missing in the conversation is a clear definition of what it even means for two structures to be "equal" in the first place.

There are three reasonable interpretations that come to mind and they are all different.

1) They are the SAME structure, as indicated by the address at which they are stored being the same.

This type of equality can be checked for by comparing the pointers to the two structures in question.

Conceivably I could define two structures as part of a union in which they could be at the same address and not be the same structure, but if I'm playing those kinds of games (and there CAN be good reasons for doing something like that) then the onus is on me to get the details right which means I'm not looking for some general way of doing things, but rather I need to have my act together and know what the hell I am doing at a very low level and take responsibility for getting it right.

2) They are bit-for-bit identical copies of each other.

This type of equality can be checked using memcmp().

3) The two structures contain the same information.

This is the tricky one.

For instance, let's say that I have a floating point value named flt in a structure and in one structure I set

a.flt = 1.0;

and in the other I set

b.flt = a.flt / 10.0 * 10.0;

All other structure elements are bitwise equal.

Do I want or need structures 'a' and 'b' to evaluate as being equal to each other?

If I do, then since they almost certainly are NOT bitwise equal, I need to check for member-wise equality and, in doing so, properly determine if two floating point values are sufficiently close as to be considered "equal".
No float involved, but my OP was intend to ask how to do point 3.
 

WBahn

Joined Mar 31, 2012
30,088
No float involved, but my OP was intend to ask how to do point 3.
While you can certainly ignore the complications associated with floating point values (or any other type that is not used in this particularly structure), just be sure to keep in mind the limitations you are imposing on the validity of your solution. In other words, don't get a solution to THIS particular problem working and then, later, assume that you can just blindly use that solution to compare ANY two structures for equality.

If you want to compare for equality in the sense of option three, then use that as your starting point. IF you can conclude that this can be reduced to using memcmp for THIS structure, that's fine. Just recognize that it is a special case that does NOT go the other way.

In general, any time a structure includes either a floating point data type or a pointer to anything, you can't use memcmp.

EDIT: Typos.
 
Last edited:

miniwinwm

Joined Feb 2, 2018
68
In general, any time a structure includes either a floating point data type or a pointer to anything, you can't use memcmp.
Quite easy to break memcmp comparison of structs containing bitfields, even when the total number of bits in the struct falls on a machine word boundary. No floats or pointers required...

Code:
#include <string.h>
#include <stdio.h>

typedef struct
{
  unsigned b1:31;
  unsigned b2:2;
  unsigned b3:31;
} c;

int main(int argc, char* argv[])
{
  c c1 = {0U, 0U, 0U};
  c c2 = {0U, 0U, 0U};

  if (memcmp(&c1, &c2, sizeof(c)) == 0)
  {
    printf("same\n");
  }
  else
  {
    printf("different\n");
  }
  return 0;
}

Output:

different
Code:
#include <string.h>
#include <stdio.h>

typedef struct
{
  unsigned b1:31;
  unsigned b2:2;
  unsigned b3:31;
} c;

c c1 = {0U, 0U, 0U};
c c2 = {0U, 0U, 0U};

int main(int argc, char* argv[])
{
  if (memcmp(&c1, &c2, sizeof(c)) == 0)
  {
    printf("same\n");
  }
  else
  {
    printf("different\n");
  }
  return 0;
}

Output:

same
This kind of behaviour is why it's not a good idea.
 
Last edited:

WBahn

Joined Mar 31, 2012
30,088
Quite easy to break memcmp comparison of structs containing bitfields, even when the total number of bits in the struct falls on a machine word boundary. No floats or pointers required...
Note that I wasn't saying that a float or pointer was REQUIRED, merely giving a couple of examples where they are SUFFICIENT.

I haven't done very much with bitfields at all, so I don't know what how they can break a memcmp() based comparison or how subtle/bltant such breakage is.

When I get a chance I'll be interested to look at your example code closely.
 

MrSoftware

Joined Oct 29, 2013
2,202
A problem that this code can have is that it assumes there are no padding bytes in there. What if the compiler pads the structure elements to, say, 8 byte boundaries and your data type is 4 bit each. You are essentially doing a sloppy memcmp operation.
As @WBahn mentions, I have personally debugged production code with this exact issue; iterating through a block of memory by incrementing a pointer instead of accessing the fields by name. Low and behold there was padding where the original code author didn't expect and the comparisons were failing. It becomes even more of a problem if you're writing a library to be used by other people, who will be allocating the memory in their code and passing a reference in to your code. You have no idea what their compiler settings will be, so you have to be sure to explicitly declare the padding (#pragma) around the data types in your header, otherwise you could be receiving references to memory that isn't laid out at all like you are expecting.

Also if you're declaring bit fields, it's a good idea to declare the unused bits as "reserved" with a specific value, instead of leaving them just as mystery bits. At least finish the byte. For example if you need 2 bits, declare the other 6 as "reserved" and specify that they must be 0. Like this:

Code:
union
{
    struct
    {
        Uint16 Bit1:1;
        Uint16 Bit2:1;
        Uint16 Reserved:14;  /* Must be 0 */
    }MyBits;
    Uint16 TwoBytes;
}MyUnion;
This won't make up for bad code, but makes things easier when debugging.
 
Last edited:

miniwinwm

Joined Feb 2, 2018
68
Also if you're declaring bit fields, it's a good idea to declare the unused bits as "reserved" with a specific value, instead of leaving them just as mystery bits. At least finish the byte. For example if you need 2 bits, declare the other 6 as "reserved" and specify that they must be 0. Like this:

Code:
union
{
    struct
    {
        Uint16 Bit1:1;
        Uint16 Bit2:1;
        Uint16 Reserved:14;  /* Must be 0 */
    }MyBits;
    Uint16 TwoBytes;
}MyUnion;
This won't make up for bad code, but makes things easier when debugging.
I can get that to break too :)

Code:
#include <string.h>
#include <stdio.h>

typedef union
{
    struct
    {
        unsigned BitField1:30;
        unsigned BitField2:30;
        unsigned Reserved:4;  /* Must be 0 */
    } MyBits;
 
    unsigned long long EightBytes;
} MyUnion;

int main(int argc, char* argv[])
{
  MyUnion c1;
  MyUnion c2;
  c1.EightBytes = 0ULL;
  c2.EightBytes = 0ULL;

  if (memcmp(&c1, &c2, sizeof(MyUnion)) == 0)
  {
    printf("same\n");
  }
  else
  {
    printf("different\n");
  }
  return 0;
}

Output:

different
 

MrSoftware

Joined Oct 29, 2013
2,202
@miniwinwm you stuffed a lot of extra bytes into the bit struct! lol

Edit --> To be the devils advocate, the memcmp should work if every bit in the union was explicitly defined, and the struct was packed.
 

miniwinwm

Joined Feb 2, 2018
68
@miniwinwm you stuffed a lot of extra bytes into the bit struct! lol
Yes, I know I did, but I don't have an 8 bit processor to test anything on here with me. It would be interesting to try this construction on a PIC or similar...

Code:
typedef union
{
    struct
    {
        unsigned BitField1:6;
        unsigned BitField2:6;
        unsigned Reserved:4;  /* Must be 0 */
    } MyBits;
    unsigned int TwoBytes;
} MyUnion;
My ARM example above will need translating into PIClish. I don't have a PIC dev environment with me so if I tried to do it I would only be guessing. Anyone want to try it?
 
Last edited:

miniwinwm

Joined Feb 2, 2018
68
To be the devils advocate, the memcmp should work if every bit in the union was explicitly defined, and the struct was packed.
Like this? :)

Code:
#include <string.h>
#include <stdio.h>

#pragma pack(1)
typedef union
{
    struct
    {
        unsigned a:1; unsigned b:1; unsigned c:1; unsigned d:1; unsigned e:1; unsigned f:1; unsigned g:1; unsigned h:1; unsigned i:1; unsigned j:1; unsigned k:1;
        unsigned Reserved:22;
    } MyBits;

    unsigned int FourBytes;
} MyUnion;

int main(int argc, char* argv[])
{
  MyUnion c1;
  MyUnion c2;
  c1.FourBytes = 0UL;
  c2.FourBytes = 0UL;

  if (memcmp(&c1, &c2, sizeof(MyUnion)) == 0)
  {
    printf("same\n");
  }
  else
  {
    printf("different\n");
  }
  return 0;
}

Output:

different
 
Last edited:
Top