Code works, but throws Incompatible Pointer Type warning

Question

Trying things out as I learn C code, I wanted to test something. It worked as expected, but throws the warning

Warning 1 assignment from incompatible pointer type [enabled by default]

The code is simple. All I am doing here is toggling PIN B7 on an atmega2560. I have a LED hooked to it and I can see it blinking, so I know it works as expected.

Can anyone explain why I am seeing this error, even though it is executed as expected? The code is as follows:

#include <avr/io.h>
#include <util/delay.h>

void main(void) {
    int *ptr;
    ptr = &PORTB; // This line throws the warning

    DDRB = (1 << 7);

    while(1) {
        *ptr = (1 << 7);
        _delay_ms(1000);

        *ptr = (0 << 7);
        _delay_ms(1000);
    }
}

PORTB is an 8bit register that has a bit per pin to control whether that pin is HIGH or LOW.

For now, I am glad it works. But these warnings annoy me.

Sejm

Answer 1

int *ptr;
ptr = &PORTB; // This line throws the warning

PORTB is a volatile unsigned char defined with something like this:

*(volatile unsigned char *) 0xBEEF

Change your ptr declaration to a volatile unsigned char *:

volatile unsigned char *ptr;
ptr = &PORTB;

Brynnbrynna

It's worth mentioning that this is the reason why declaring another pointer is superfluous - assigning to PORTB directly will work just fine. – Veloz

@H2CO3 in this case yes, but he may want to pass a port as an argument to a function and he would have to use volatile unsigned char * as the parameter type. – Brynnbrynna

@Brynnbrynna Hmm, your answer is admittedly more correct than my original post and by the time I fixed it you already had this up. I've referenced you and I'll upvote your answer as well. – Scauper

Answer 2

Edit 2:

By the time I got around to correcting my original answer (below) I saw that @ouah had already posted a correct answer so I ask you to please use it instead of mine. For a more detailed explanation please read 'Edit 1' below.

My original answer:

PORTB is probably not defined as an int which means that when you take the address of PORTB you won't get an int *. If you are sure you're right and that you can use an int * for PORTB you can use a cast to tell the compiler that it doesn't need to worry. You can cast it like this:

ptr = (int*)&PORTB

Go to the definition of PORTB and let us know what type it is.

Edit 1:

My assumption was that you knew that PORTB was an int and that you're casting it from void*. Thanks @H2CO3 for pointing out that it is in fact a (*(volatile uint8_t *)(0x25)) which basically means that PORTB is a volatile uint8_t.

This being the case you should NEVER cast it to an int! If this worked it means that your machine is probably little endian and you shouldn't rely on this.

To explain it properly let's set up a simple example:

We have this in memory:

Address  Value
0x02        7
0x03       11

Note: 7 is 0x07 in hex and 00000111 in binary and 11 is 0x0B in hex and 00001011 in binary

Now we have 2 pointers:

uint8_t* BytePointer;
int*     IntPointer; // int is either 2 or 4 bytes, we will assume it is 2 bytes.
int Value16;
uint8_t Value8;

// Let's set both to point to our fake memory address
BytePointer = (uint8_t*) 0x02;
IntPointer  = (int*)     0x02;

// Now let's see what value each holds?
Value8 = *BytePointer;
Value16 = *IntPointer;

// Value8 will contain 0x07
// Value16 will contain 0x0B07 on a Little Endian machine
// Value16 will contain 0x070B on a Big Endian Machine

This example illustrates what happens when we read values from pointers, what about writing? Let's keep the same variables as before and write some values

*BytePointer = 5;

The memory would now look like this:

0x02     5
0x03    11

What about an int pointer?

*IntPointer = 5;

Since an int is two bytes it will alter two bytes:

// Little endian
0x02     5
0x03     0

// Big endian
0x02     0
0x03     5

So if you use PORTB as an int, every time you assign to it you are writing 2 bytes, one to the address of PORTB and one straight after. I hope whatever is after isn't important... You shouldn't do this so you should use an uint8_t* if you really want to use pointers.

But by the time I got around to explaining it all properly @ouah has already posted a correct answer. Please refer to it for how this should be done.

Scauper

@nonsensical NAH NO NO NO PLEASE NOOOOOO! PORTB is a volatile uint8_t *. It's a macro from the AVR toolchain. int is 2 bytes long on the AVR platform. This will write stuff to memory it's not supposed to write to! – Veloz

I looked up the define for PORTB which is _SFR_IO8(0x05) which is a macro for _MMIO_BYTE((0x05) + 0x20) which in turn is a macro for (*(volatile uint8_t *)(0x25)). I changed my int *ptr to volatile uint8_t *ptr and that works as well. Just had to dig a little deeper before posting my question. :) Thanks again! – Sejm

@JoeyvanHummel Yes, and you don't even need the pointer - assigning directly to PORTB will work (and it's preferred for obvious readability reasons). – Veloz

Ah yes, I agree on that. I was simply testing a few things to learn more, and was bugged that I made an error and wanted to learn what my error was. Thank you, though. :) – Sejm

@H2CO3 Yup, you're very right. I was writing an improved answer after posting the first quick and dirty version. Sorry. But since you mentioned it, I'll include the information about what it means to cast the types. – Scauper

@JoeyvanHummel You're welcome but H2CO3 is right. Casts force the compiler to trust you, which means that if you are wrong then the mistake goes unnoticed. I've written an explanation as to why ouah's answer below is more correct than mine. – Scauper

I understand; it has been done. Thank you for your elaborate edits, though. I've learned a lot! – Sejm