bit fields warning help, AS6.1 + Naggy

Go To Last Post
13 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
3485 : cast to 'volatile bit_field *' from smaller integer type 'uint16_t' (aka 'unsigned short')

I have hundreds of this warning.

typedef struct 
{ 
uint8_t bit0 : 1, 
bit1 : 1, 
bit2 : 1, 
bit3 : 1, 
bit4 : 1, 
bit5 : 1, 
bit6 : 1, 
bit7 : 1; 
} bit_field;

#define	button1 (*(volatile bit_field *) (_SFR_ADDR(PINA))).bit0

the warnings point to every instance I use the defined ports

if (button1 == HIGH) {
...
}

led1 = LOW;

etc...

as far as I understand, it is warning me that I am casting to a larger data type from a smaller data type?
Or does the warning come from the fact I am casting to a pointer to a struct?
I know it is just a warning but some warnings are very useful and I don't want to ignore them.

Also, I am using AS6.1 with Naggy extension.
I am building this for m2560.
And these are the compile options (which are default, i think)

-funsigned-char -funsigned-bitfields -Os -ffunction-sections -fpack-struct -fshort-enums -g2 -Wall -mmcu=atmega2560 -c -std=gnu99 -MD -MP -MF "$(@:%.o=%.d)" -MT"$(@:%.o=%.d)" -MT"$(@:%.o=%.o)" 

Also, I cannot recall if I have seen these errors when I have not yet installed Naggy. I don't know if it is Naggy who generates these.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I tried to disable Naggy, and indeed the warnings disappeared.
What does this mean? Is Naggy wrong to point out those warnings?
Is avr-gcc wrong to NOT point them out?

Not knowing the context, is it really dangerous to cast a smaller variable size to a larger one?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

You have commas instead of semicolons in your struct definition.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I tried this one yesterday.

typedef struct 
{ 
uint8_t bit0 : 1;
uint8_t bit1 : 1;
uint8_t bit2 : 1;
uint8_t bit2 : 1;
uint8_t bit3 : 1;
uint8_t bit4 : 1;
uint8_t bit5 : 1;
uint8_t bit6 : 1;
} bit_field;

I don't know the difference of that with the original one.
The warnings still persist. And I am still no wiser about the warning message.

3485 : cast to 'volatile bit_field *' from smaller integer type 'uint16_t' (aka 'unsigned short') 
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Bit fields are fine as long as you don't depend on their arrangement/allocation inside the larger datatypes. I like to use them, for example, to compact the storage of data structures used as elements of const data tables. The initializer list for such a table only needs to be written to match the bitfield-containing structure's declaration, regardless of whether the compiler likes to allocate bitfields in a little- or big-endian way. If you go trying to overlay (by casting - a disgusting practice in its own right) an "eight bitfield struct" onto a specific piece of hardware (like an eight-bit I/O port), though, you WILL be depending on the particular endianness of the compiler, and 'bang' goes portability.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I found this particular code snippet here in the forums and I thought it was a cool way of giving an alias to a port.

I understand the issue about portability and endianness, but could you please explain the warning message. And also your comment about

Quote:
by casting - a disgusting practice in its own right

which is obviously what I am doing here.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

For OP's original code... the struct contains constants. Better to use an enum, eh?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

does it?
I think the struct means that I am creating eight 1bit-wide members.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Author here - looks like an incorrect warning off the top of my head. Clang is likely emitting this warning because it thinks a pointer is bigger than uint16_t (true for other architectures like x86, not true for most AVRs). You can make Naggy shut up about this specific diagnostic by adding it to the blacklist (see https://github.com/saaadhu/naggy... )

Regards

Senthil

 

blog | website

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Sorry but why would you be using _SFR_ADDR(PINA)? The PINA macro itself is already define in terms of _SFR_ADDR().

Oh and when I copy the struct definition you show most recently Naggy (helpfully!) points out that there are two copies of bit2 in it!

The following has no Naggy or compiler/linker errors or warnings:

#include 

typedef struct
{
	uint8_t bit0 : 1;
	uint8_t bit1 : 1;
	uint8_t bit2 : 1;
	uint8_t bit3 : 1;
	uint8_t bit4 : 1;
	uint8_t bit5 : 1;
	uint8_t bit6 : 1;
} bit_field;

#define   button1 (*(volatile bit_field *) (&PINA)).bit0 
	
int main(void) {
	if (button1 == 1) {
		PORTB = 0x55;
	}
}

it generates:

0000009a 
: int main(void) { if (button1 == 1) { 9a: c8 9b sbis 0x19, 0 ; 25 9c: 02 c0 rjmp .+4 ; 0xa2 PORTB = 0x55; 9e: 85 e5 ldi r24, 0x55 ; 85 a0: 88 bb out 0x18, r24 ; 24 } } a2: 80 e0 ldi r24, 0x00 ; 0 a4: 90 e0 ldi r25, 0x00 ; 0 a6: 08 95 ret

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
Oh and when I copy the struct definition you show most recently Naggy (helpfully!) points out that there are two copies of bit2 in it!

Aha! You are right of course. I typed that second bit manually. I know, I'm supposed to copy-paste the actual code (which i did in the OP, btw). But I was away from my computer at the time. Anyway sorry about that bit.

Quote:
Sorry but why would you be using _SFR_ADDR(PINA)? The PINA macro itself is already define in terms of _SFR_ADDR().

Hmmm. I have no idea. I just copied the code snippet somewhere here in the forums. Thought it was cool, tried to use it, it worked, no problem.
I will try this tomorrow when I get to my workstation.
Thanks for trying it out!

Quote:
Clang is likely emitting this warning because it thinks a pointer is bigger than uint16_t (true for other architectures like x86, not true for most AVRs).

This was my thought initially, but it would seem clawson has fixed it. I will report back tomorrow.
Thanks saaadhu for the link. And thank you for Naggy!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

Hmmm. I have no idea. I just copied the code snippet somewhere here in the forums. Thought it was cool, tried to use it, it worked, no problem.
I will try this tomorrow when I get to my workstation.
Thanks for trying it out!

This is the problem with picking up code from the internet. You never really know if you are looking at a work of utter genius or a complete pile of shite. I'd tend to go by recommendations of others who either have experience of something they considered good or those that know genius when they see it. Personally I like anything by a contributor here called "danni" and would recommend his sbit.h which is to be found on page 5 (is it?) of the "bit manipulation 101" thread in tutorial forum. I won't recount the entire thing here but it starts..

#ifndef _sbit_h_
#define _sbit_h_

#include 

//                      Access bits like variables:
struct bits {
  uint8_t b0:1, b1:1, b2:1, b3:1, b4:1, b5:1, b6:1, b7:1;
} __attribute__((__packed__));
#define SBIT_(port,pin) ((*(volatile struct bits*)&port).b##pin)
#define SBIT(x,y)       SBIT_(x,y)

#ifdef PORTA

#define	PORT_A0		SBIT( PORTA, 0 )
#define	PORT_A1		SBIT( PORTA, 1 )
#define	PORT_A2		SBIT( PORTA, 2 )
etc.

You can then simply:

#define button1 PIN_A0

then

int main(void) {
   if (button1 == 1) {
      PORTB = 0x55;
   }
} 

as before. But by a rather circuitous route you've effectively re-invented this anyway ;-)

(oh and convention says it should be "BUTTON1" not "button1". The use of all upper case tells the reader to be on the lookout for a macro definition).

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Yep. The extra _SFR_ADDR did cause the warnings.
Thank you for the help clawson!
Had I not used Naggy I would have never known about this.
Thanks saaadhu for a great extension.

Quote:
I'd tend to go by recommendations of others who either have experience of something they considered good or those that know genius when they see it.

And with that, I will be taking your recommendation and use danni's sbit.h :wink: