warning

Go To Last Post
43 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
// variable
static char menu_p[][FONT_NORMAL_SIZE+1] PROGMEM = {
  "Line 1              ",
  "Line 2              ",
  "Line 3              ",
  "Line 4              ",
  "Line 5              "
};
//prototype
signed char active_menu (char argc, char *argv[FONT_NORMAL_SIZE+1]);
// call
active_menu ((sizeof(menu_p)/(FONT_NORMAL_SIZE+1)), menu_p);

Quote:
warning: passing arg 2 of `active_menu' from incompatible pointer type

What is wrong?

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

you are attempting to pass a PROGMEM space pointer as a DATA space pointer.

brberie wrote:
static char menu_p[][FONT_NORMAL_SIZE+1] PROGMEM = ...

brberie wrote:
signed char active_menu (char argc, char *argv[FONT_NORMAL_SIZE+1]);

Writing code is like having sex.... make one little mistake, and you're supporting it for life.

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

so?

signed char active_menu (char argc, const char *argv[FONT_NORMAL_SIZE+1]);

won't help!

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

Maybe this:

signed char active_menu (char argc, char argv[][FONT_NORMAL_SIZE+1]);
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

It's a subtle thing.

Your function prototype is looking for an array of (FONT_NORMAL_SIZE+1) pointers to char's.
Try this:

//prototype
signed char active_menu (char argc, char (*argv)[FONT_NORMAL_SIZE+1]); 

Note the extra set of parentheses.

Now the function is looking for a pointer to a datatype that consists of an array of FONT_NORMAL_SIZE+1 chars.

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

there is no difference between array and pointer. It shouldn't help

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

brberie wrote:
so?
signed char active_menu (char argc, const char *argv[FONT_NORMAL_SIZE+1]);

won't help!

CONST does NOT change the dataspace. It only declares the value as unchageable by the routine.

read the documentation on pgmspace

Writing code is like having sex.... make one little mistake, and you're supporting it for life.

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

lfmorrison wrote:
It's a subtle thing.

Your function prototype is looking for an array of (FONT_NORMAL_SIZE+1) pointers to char's.
Try this:

//prototype
signed char active_menu (char argc, char (*argv)[FONT_NORMAL_SIZE+1]); 

Note the extra set of parentheses.

Now the function is looking for a pointer to a datatype that consists of an array of FONT_NORMAL_SIZE+1 chars.


1. I didn't really understand
2. didn't help.

What does help is

active_menu ((sizeof(menu_p)/FONT_NORMAL_SIZE), (char**)menu_p)

But I don't understand why should i cast?!

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

Actually lfmorrison is correct as well.

the way your prototype is declared it's asking for an array of pointers to char, that is FONT_NORMAL_SIZE+1 in length. Instead of a array of pointers to arrays of chars that are FONT_NORMAL_SIZE+1 in length.

When you turn one of the dimensions of a multi dimensional array into a pointer like that, it works back to front, that is it's the last index value that is replaced by the '*' in the front. And so on...

in your case

char menu_p[][FONT_NORMAL_SIZE+1];

is the same as (in terms of passing as a pointer)

char *menu_p[];

which is the same as (in terms of passing as a pointer)

char **menu_p;

Writing code is like having sex.... make one little mistake, and you're supporting it for life.

Last Edited: Wed. Oct 31, 2007 - 04:38 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

brberie wrote:

What does help is

active_menu ((sizeof(menu_p)/FONT_NORMAL_SIZE), (char**)menu_p)

But I don't understand why should i cast?!

You shouldn't. It works because the cast is forcibly removing the PGMSPACE modifier from the pointer. But it is wrong, unless you cast it back into a PGMSPACE pointer before use.

Writing code is like having sex.... make one little mistake, and you're supporting it for life.

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

brberie wrote:
What does help is

active_menu ((sizeof(menu_p)/FONT_NORMAL_SIZE), (char**)menu_p)

But I don't understand why should i cast?!

But that formulation doesn't have the correct syntactic meaning, and although it may compile without warnings, it *will* fail in practise.

That formulation describes a situation in which there is one explicit table of pointers, and each of those pointers points to a string of chars elsewhere in memory. If you attempt to dereference the pointer, the function will attempt to use the pointer as the starting point for a table of pointers to strings. It will try to fetch a second-layer explicit pointer as the result of dereferences the first pointer, and then use that second-layer pointer to gain access to the strings themselves. But in memory, there physically isn't a second layer of explicit pointers to be fetched - there is simply a large two-dimensional array of chars.

Either my formulation, or CirMicro's, ought to compile cleanly and (hopefully ;) ) work correctly.

Last Edited: Wed. Oct 31, 2007 - 04:45 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Did you try writing as I did above?

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

Quote:
6.5.2 Define Documentation
6.5.2.1 #define PGM_P const prog_char *
Used to declare a variable that is a pointer to a string inprogram space.

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

CirMicro wrote:
Did you try writing as I did above?

1. It shouldn't help
2. but I tried and it didn't

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

lfmorrison wrote:
brberie wrote:
What does help is

active_menu ((sizeof(menu_p)/FONT_NORMAL_SIZE), (char**)menu_p)

But I don't understand why should i cast?!

But that formulation doesn't have the correct syntactic meaning, and although it may compile without warnings, it *will* fail in practise.

The function will attempt to fetch a second explicit pointer when it dereferences the first pointer. But in memory, there physically isn't a second layer of explicit pointers to be fetched - there is simply a large two-dimensional array of chars.

Either my formulation, or CirMicro's, ought to compile cleanly and (hopefully ;) ) work correctly.

I've tried -> warning still in place!

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

Strange, I just compiled your code in a program I have and it gives the error that you mentioned. I changed the declaration to what I suggested and the warning went away.

You did remove the casts, correct?

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

brberie wrote:
Quote:
6.5.2 Define Documentation
6.5.2.1 #define PGM_P const prog_char *
Used to declare a variable that is a pointer to a string inprogram space.

yes but you also need to look at the typedef for prog_char which is:

typedef char prog_char PROGMEM;

Writing code is like having sex.... make one little mistake, and you're supporting it for life.

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

brberie wrote:
I've tried -> warning still in place!

No offence, but either you're lying, or you aren't giving us the whole story.

Same story as CirMicro: I have tried your original formulation, got the worning, then I tried both CirMicro's formulation and my own, and got no warnings at all.

Here's my complete test case so that others can reproduce it:

#include 

#define FONT_NORMAL_SIZE 20
// variable
static char menu_p[][FONT_NORMAL_SIZE+1] PROGMEM = {
  "Line 1              ",
  "Line 2              ",
  "Line 3              ",
  "Line 4              ",
  "Line 5              "
};


//prototype
signed char active_menu (char argc, char *argv[FONT_NORMAL_SIZE+1])
{
	return 'd';
}

//prototype
signed char active_menu1 (char argc, char (*argv)[FONT_NORMAL_SIZE+1])
{
	return 'd';
}

//prototype
signed char active_menu2 (char argc, char argv[][FONT_NORMAL_SIZE+1])
{
	return 'd';
}

// call
int main(void)
{
	active_menu ((sizeof(menu_p)/(FONT_NORMAL_SIZE+1)), menu_p);
	active_menu1 ((sizeof(menu_p)/(FONT_NORMAL_SIZE+1)), menu_p);
	active_menu2 ((sizeof(menu_p)/(FONT_NORMAL_SIZE+1)), menu_p);
    return 0;
}

[edit]
Note: Compiler suite is WinAVR 20070525.

Last Edited: Wed. Oct 31, 2007 - 05:00 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

CirMicro wrote:
Strange, I just compiled your code in a program I have and it gives the error that you mentioned. I changed the declaration to what I suggested and the warning went away.

You did remove the casts, correct?

refer to my original answer.

the original array is declared to exist in FLASH via the "PROGMEM" attribute.

The function prototype (no matter which version so far) does not include the progmem attribute for the pointer, therefore the pointers are incompatible, as they exist in different address spaces. (using a cast forcibply strips the address space qualifier, and thus removes the warning... the compiler assumes you know what you are doing when you cast)

Writing code is like having sex.... make one little mistake, and you're supporting it for life.

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

Quote:
The function prototype (no matter which version so far) does not include the progmem attribute for the pointer, therefore the pointers are incompatible, as they exist in different address spaces. (using a cast forcibply strips the address space qualifier, and thus removes the warning... the compiler assumes you know what you are doing when you cast)

I'm not using any casts. If you reread it I was making sure he removed the casts that he put there previously, otherwise he will still receive a warning.

The compiler isn't complaining about the array being in flash. The "pointer" is still compatible no matter where the content is. Of course how you access it afterwards would surely make a difference.

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

CirMicro wrote:
The compiler isn't complaining about the array being in flash. The "pointer" is still compatible no matter where the content is. Of course how you access it afterwards would surely make a difference.

No they are not compatible. The compiler looks at every aspect of the pointer, including memory spaces to determine compatibility. So EVEN if he used the same declaration, less the PROGMEM attribute, he would still get the incompatible pointer types warning. (or a discarding qualifier without cast warning)

The reason the compiler gives a warning, is that there is no way at runtime to determine what address space points to, so the compiler does it's best to tell you, when you are assigning a value from a pointer in one space, to one that refers to another.

Go ahead, try it, and let me know what happens.

(BTW, I realize your example was not using casts, was merely trying to explain why the warning goes away when you do use the cast)

Writing code is like having sex.... make one little mistake, and you're supporting it for life.

Last Edited: Wed. Oct 31, 2007 - 05:14 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Glitch,

In WinAVR 20070525, at least, the presence or absence of the PROGMEM attribute really doesn't change anything in regards to this particular compiler warning in any way.

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

lfmorrison wrote:
Glitch,

In WinAVR 20070525, at least, the presence or absence of the PROGMEM attribute really doesn't change anything in regards to this particular compiler warning in any way.

Well there are 2 errors here, one is what you pointed out, with the form of the pointer, which would definitely generate the warning the OP mentioned. the 2nd problem is the address space one, which will also generate a warning, though it may be a "discarding qualifier without cast" warning instead of the "incompatible pointer type" warning.

Writing code is like having sex.... make one little mistake, and you're supporting it for life.

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

Quote:
Go ahead, try it, and let me know what happens.

I did try and it worked fine, same as it did for lfmorrison.

I think you're thinking that his "pointer" is in FLASH, it is in SRAM, while the address it points to is in FLASH.

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

I'm using winavr 20060421

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

So, I've expanded my test case somewhat to illustrate what I've seen so far:

#include 
#include 

#define FONT_NORMAL_SIZE 20
// variable
static char menu_p[][FONT_NORMAL_SIZE+1] = {
  "Line 1              ",
  "Line 2              ",
  "Line 3              ",
  "Line 4              ",
  "Line 5              "
};

static char menu_p1[][FONT_NORMAL_SIZE+1] PROGMEM = {
  "Line 1              ",
  "Line 2              ",
  "Line 3              ",
  "Line 4              ",
  "Line 5              "
};

const static char menu_p2[][FONT_NORMAL_SIZE+1] = {
  "Line 1              ",
  "Line 2              ",
  "Line 3              ",
  "Line 4              ",
  "Line 5              "
};

const static char menu_p3[][FONT_NORMAL_SIZE+1] PROGMEM = {
  "Line 1              ",
  "Line 2              ",
  "Line 3              ",
  "Line 4              ",
  "Line 5              "
};


//prototype
signed char active_menu (char argc, char *argv[FONT_NORMAL_SIZE+1])
{
   return 'd';
}

//prototype
signed char active_menu1 (char argc, char (*argv)[FONT_NORMAL_SIZE+1])
{
   return 'd';
}

//prototype
signed char active_menu2 (char argc, char argv[][FONT_NORMAL_SIZE+1])
{
   return 'd';
}

// call
int main(void)
{
   // Test 1
   active_menu ((sizeof(menu_p)/(FONT_NORMAL_SIZE+1)), menu_p);
   // Test 2
   active_menu1 ((sizeof(menu_p)/(FONT_NORMAL_SIZE+1)), menu_p);
   // Test 3
   active_menu2 ((sizeof(menu_p)/(FONT_NORMAL_SIZE+1)), menu_p);

   // Test 4
   active_menu ((sizeof(menu_p1)/(FONT_NORMAL_SIZE+1)), menu_p1);
   // Test 5
   active_menu1 ((sizeof(menu_p1)/(FONT_NORMAL_SIZE+1)), menu_p1);
   // Test 6
   active_menu2 ((sizeof(menu_p1)/(FONT_NORMAL_SIZE+1)), menu_p1);

   // Test 7
   active_menu ((sizeof(menu_p2)/(FONT_NORMAL_SIZE+1)), menu_p2);
   // Test 8
   active_menu1 ((sizeof(menu_p2)/(FONT_NORMAL_SIZE+1)), menu_p2);
   // Test 9
   active_menu2 ((sizeof(menu_p2)/(FONT_NORMAL_SIZE+1)), menu_p2);

   // Test 10
   active_menu ((sizeof(menu_p3)/(FONT_NORMAL_SIZE+1)), menu_p3);
   // Test 11
   active_menu1 ((sizeof(menu_p3)/(FONT_NORMAL_SIZE+1)), menu_p3);
   // Test 12
   active_menu2 ((sizeof(menu_p3)/(FONT_NORMAL_SIZE+1)), menu_p3);

   return 0;
}

Result, using WinAVR 20070525:
Test 1 yields an "incompatible pointer type" warning because the type of data being pointed to is a mismatch - you're being passed a pointer to the first entry of an array consisting of elements which, themselves, are 21-byte arrays, but the function is expecting a pointer to a pointer-sized data type.

Test 2 passes with absolutely no warnings at all.
Test 3 passes with absolutely no warnings at all.

Test 4 yields a warning for the same reason as Test 1 did.

Test 5 passes with absolutely no warnings at all.
Test 6 passes with absolutely no warnings at all.

Tests 7 through 12 all fail with the "incompatible pointer type" warning, because the "const" really is important to the way in which the compiler limits the scope of the targets of pointers.

Tests 7 and 10 have another factor contributing to the same warning, namely, for the same reason tests 1 and 4 failed.

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

brberie wrote:
I'm using winavr 20060421

Okay, I'll try and download that version on a clean PC to see how it responds to all this.

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

CirMicro wrote:
Quote:
Go ahead, try it, and let me know what happens.

I did try and it worked fine, same as it did for lfmorrison.

I think you're thinking that his "pointer" is in FLASH, it is in SRAM, while the address it points to is in FLASH.

No I am not thinking the pointer is in FLASH, it is the data that is in FLASH.

Are you saying that GCC does not emit any sort of warning when you assign the value from a PGM_P pointer to one that is (assumed to be) pointing to RAM? This is a big problem in my eyes, as there is no way to determine the address space at runtime. So the programmer must rely on the compiler to let him know when he has inadvertantly switched pointer domains.

I understand that it's all how you use the pointer, in the end. But without the warning it would be easy to accidentally use the pointer the wrong way, creating a bug that could be very hard to find.

Writing code is like having sex.... make one little mistake, and you're supporting it for life.

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

I tried again

signed char active_menu (char argc, char argv[][FONT_NORMAL_SIZE+1]

The same issue:(
Could you try

//prototype
signed char active_menu2 (char argc, char i, char argv[][FONT_NORMAL_SIZE+1])
{
   return (i<= FONT_NORMAL_SIZE)?argv[argc-1][i]:0;
}

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

Just another random thought... perhaps that particular warning (that I'm thinking about) is switched off by default... try "-Wall"

Writing code is like having sex.... make one little mistake, and you're supporting it for life.

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

Quote:
Just another random thought... perhaps that particular warning (that I'm thinking about) is switched off by default... try "-Wall"

I have -Wall in my make file, but no warnings. I'm using WinAVR 20070525.

I think the problem is his declaration doesn't specify that the pointer is constant only that it points to constant data. So you could legally change what the pointer points to.

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

glitch wrote:
brberie wrote:
Quote:
6.5.2 Define Documentation
6.5.2.1 #define PGM_P const prog_char *
Used to declare a variable that is a pointer to a string inprogram space.

yes but you also need to look at the typedef for prog_char which is:

typedef char prog_char PROGMEM;

signed char active_menu (char argc, prog_char argv[][FONT_NORMAL_SIZE+1]) {

The same warning :(

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

try "const prog_char" (although the addition of "const" shouldn't be required here)

Writing code is like having sex.... make one little mistake, and you're supporting it for life.

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

Quote:
return (i<= FONT_NORMAL_SIZE)?argv[argc-1][i]:0;

This would be a problem because now you are trying to dereference argv[argc-1] as if it were in sram.

I think you want something like:

return (i<= FONT_NORMAL_SIZE)?pgm_read_byte(&argv[argc-1][i]):0;
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

CirMicro wrote:

I think the problem is his declaration doesn't specify that the pointer is constant only that it points to constant data. So you could legally change what the pointer points to.

And that should be fine... in this case it's the data that is read-only... not the pointer itself... the value of the pointer can be changed. Note that this change would be local only to the function anyway.

think of the string routines

int strcmp(const char *s1, const char *s2) {
  while((*s1) && (*s2))
  {
    if(*s1-*s2) return (*s1-*s2);
    s1++; s2++;
  }
  return (*s1-*s2);
}

The above is perfectly legal (and desired), even though the pointers are "const", and we modify them.

Writing code is like having sex.... make one little mistake, and you're supporting it for life.

Last Edited: Wed. Oct 31, 2007 - 06:09 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
signed char active_menu (char argc, const char argv[][FONT_NORMAL_SIZE+1])

that works as well as

signed char active_menu (char argc, const char (*argv)[FONT_NORMAL_SIZE+1])
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

CirMicro wrote:
Quote:
return (i<= FONT_NORMAL_SIZE)?argv[argc-1][i]:0;

This would be a problem because now you are trying to dereference argv[argc-1] as if it were in sram.

I think you want something like:

return (i<= FONT_NORMAL_SIZE)?pgm_read_byte(&argv[argc-1][i]):0;


u r correct!

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

Well this does appear to be a difference between WinAVR20060421 and WinAVR20070525.

Test cases run on both machines, identical makefiles and source code.

In WinAVR 20070525, the presence/absence of PROGMEM alone does not have any effect on the warning; in WinAVR 20060421, it does.

In the mean time, it appears that some brainstorming has been going on here about how to allow the PROGMEM to coexist peacefully - I have some more reading to do before I can contribute to that part of the discussion.

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

Which seems like a good reason to take the bug fixes when they become available.

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

Interesting that the warning was dropped... wonder why? It might be a bug that should be reported (I'd say it's a bug if it's not emitting a warning) Can anyone confirm that it is indeed a bug, or is this the desired behaviour now?

Writing code is like having sex.... make one little mistake, and you're supporting it for life.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
int strcmp(const char *s1, const char *s2)

Actually the pointers are not const here either, but what it points to is constant.

If you tried to add something like: *s1++ you would get an error. If you changed the definition to:

int strcmp(const char *const s1, const char * const s2)

Then the pointers would be constant an throw an error for the s1++ operations.

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

indeed... I thought that was the behaviour you thought it would be, from your post. Sorry if I misunderstood.

Writing code is like having sex.... make one little mistake, and you're supporting it for life.

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

Quote:
Interesting that the warning was dropped... wonder why? It might be a bug that should be reported (I'd say it's a bug if it's not emitting a warning) Can anyone confirm that it is indeed a bug, or is this the desired behaviour now?

After thinking about it I think you are correct that this should still give a warning. I don't have time right now to shuffle through the faqs. I do have -99 compliance enabled. I wonder if the rules for this may have changed?