8

I have started today to program on a PIC16f88, and found that the header for its registers contains a union that only contains a struct:

extern volatile unsigned char ANSEL __at(0x09B);
typedef union {
    struct {
        unsigned ANS0       :1;
        unsigned ANS1       :1;
        unsigned ANS2       :1;
        unsigned ANS3       :1;
        unsigned ANS4       :1;
        unsigned ANS5       :1;
        unsigned ANS6       :1;
    };
} ANSELbits_t;
extern volatile ANSELbits_t ANSELbits __at(0x09B);

Does it provide any benefits to enclose the struct inside a union that only contains that struct?

Its access I guess is going to be exactly the same as if it were a simple struct (because the struct is anonymous):

ANSELbits.ANS4 = 0;

2 Answers2

2

There is no benefit in standard portable C.

But code like this is used to circumvent (in a non-portable way) all the type checking that your C compiler will make.

You are then empowered to set all the members of the underlying struct in one go, which is useful in this case as it contains a lot of bit fields.

Bathsheba
  • 231,907
  • 34
  • 361
  • 483
  • 2
    any porability considerations are voided when I see `extern volatile ANSELbits_t ANSELbits __at(0x09B);` which is specific to Keil compiler used by some programmers in the uC programming (expensive and IMO not very good), but supported directly by ARM – 0___________ Jan 27 '19 at 16:08
  • @P__J__: Yes, that's a good point. It can only be to circumvent the type system then. – Bathsheba Jan 27 '19 at 16:09
  • In fact, I am using SDCC compiler. I suppose `__at` is being a macro to `@` – alx - recommends codidact Jan 27 '19 at 16:10
  • 1
    @CacahueteFrito: in which case I'm pretty convinced I'm correct. Remove the `union` and see what happens. Compiler warnings galore I'd expect. – Bathsheba Jan 27 '19 at 16:11
  • It will not. I think someone has forgotten another member, or just during the program development this member was not needed but programmer was too lazy to remove the wrapping union (very common in real life) – 0___________ Jan 27 '19 at 16:11
  • 1
    @P__J__: Trust me, this is an old-fashioned idiom. Not much more I can say really! – Bathsheba Jan 27 '19 at 16:12
  • I am using MPLAB X, because of the teacher, and because I didn't find examples using `Makefile`s for that chip, but I added SDCC because I hate proprietary compilers and SDCC was the most closely related to GCC which is what I always used. – alx - recommends codidact Jan 27 '19 at 16:13
  • Trust me - will not change anything. Same warnings. – 0___________ Jan 27 '19 at 16:14
  • For using all fields at once, it's clearly not, beacuse there is another variable that shares the same address, which is used for that (I updated the question with it just now) – alx - recommends codidact Jan 27 '19 at 16:14
  • 2
    @CacahueteFrito PIC uC? - my condolences :). LOL - the original programmer did not know how to use unions and has found this "workaround" :) – 0___________ Jan 27 '19 at 16:16
  • 3
    Using bit-fields means the code is not portable, period. Almost everything about bit-fields is implementation-defined. – Jonathan Leffler Jan 27 '19 at 16:36
  • Oh, I accidentally opened the header of XC8, so this code is not used by SDCC. However the SDCC header still has the __at(). Also, the SDCC header does have another struct inside the union, so the only possible answer is that the XC8 programmers from uCheap were too lazy to clean the code. – alx - recommends codidact Jan 27 '19 at 17:20
  • 1
    Can you give an example of code that would "set all the members of the underlying struct in one go" given this definition? I can understand how you would do that if the union had another member, but as it is, I don't understand. (I also see that given the compiler-specific `__at`, presumably you can set all the fields by assigning to `ANSEL`, but that would seem to be the case whether the `union` is there or not.) – Nate Eldredge Jan 27 '19 at 20:01
  • @Bathsheba: I'd also be interested in such an example. – Thomas Padron-McCarthy Jan 28 '19 at 07:42
2

It does not make any difference if you wrap and I suppose that someone has forgoten to add another member (or did not copy-paste everything) as in the declaration below. No warnings will be suppressed.

typedef union {
    struct {
        unsigned ANS0       :1;
        unsigned ANS1       :1;
        unsigned ANS2       :1;
        unsigned ANS3       :1;
        unsigned ANS4       :1;
        unsigned ANS5       :1;
        unsigned ANS6       :1;
    };
    uint8_t d8;
} ANSELbits_t;
extern volatile ANSELbits_t ANSELbits __at(0x09B);

BTW if the struct has to fit in 1 byte (8 bits) this declaration is wrong and uint_t type should be used instead.

0___________
  • 60,014
  • 4
  • 34
  • 74