Search code examples
avr-gcc

const static char assignment with PROGMEM causes error in avr-g++ 5.4.0


I have a piece of code that was shipped as part of the XLR8 development platform that formerly used a bundled version (4.8.1) of the avr-gcc/g++ compiler. I tried to use the latest version of avr-g++ included with by my linux distribution (Ubuntu 22.04) which is 5.4.0

When running that compiler, I am getting the following error that seems to make sense to me. Here is the error and the chunk of related code below. In the bundled version of avr-g++ that was provided with the XLR8 board, this was not an error. I'm not sure why because it appears that the code below is attempting to place 16 bit words into an array of chars.

A couple questions,

  1. Can anyone explain the reason this worked with previous avr-gcc releases and was not considered an error?
  2. Because of the use of sizeof in the snippet below to control the for loop terminal count, I think the 16 bit size was supposed to be the data type per element of the array. Is that accurate?
  3. If the size of the element was 16 bits, then is the correct fix simply to make that array of type unsigned int rather than char?
/home/rich/.arduino15/packages/alorium/hardware/avr/2.3.0/libraries/XLR8Info/src/XLR8Info.cpp:157:12: error: narrowing conversion of ‘51343u’ from ‘unsigned int’ to ‘char’ inside { } [-Wnarrowing]
      0x38BF};
bool     XLR8Info::hasICSPVccGndSwap(void)  {      
// List of chip IDs from boards that have Vcc and Gnd swapped on the ICSP header
// Chip ID of affected parts are 0x????6E00. Store the ???? part
const static char cidTable[] PROGMEM =
  {0xC88F,  0x08B7,  0xA877,  0xF437,
   0x94BF,  0x88D8,  0xB437,  0x94D7,  0x38BF,  0x145F,  0x288F,  0x28CF,
   0x543F,  0x0837,  0xA8B7,  0x748F,  0x8477,  0xACAF,  0x14A4,  0x0C50,
   0x084F,  0x0810,  0x0CC0,  0x540F,  0x1897,  0x48BF,  0x285F,  0x8C77,
   0xE877,  0xE49F,  0x2837,  0xA82F,  0x043F,  0x88BF,  0xF48F,  0x88F7,
   0x1410,  0xCC8F,  0xA84F,  0xB808,  0x8437,  0xF4C0,  0xD48F,  0x5478,
   0x080F,  0x54D7,  0x1490,  0x88AF,  0x2877,  0xA8CF,  0xB83F,  0x1860,
   0x38BF};
uint32_t chipId = getChipId();
for (int i=0;i< sizeof(cidTable)/sizeof(cidTable[0]);i++) {
   uint32_t cidtoTest = (cidTable[i] << 16) + 0x6E00;
   if (chipId == cidtoTest) {return true;}
} 
return false;
}

Solution

  • As you already pointed out, the array type char definitely looks wrong. My guess is, that this is bug that may have never surfaced in the field. hasICSPVccGndSwap should always return false, so maybe they never used a chip type that had its pins swapped and got away with it.

    Can anyone explain the reason this worked with previous avr-gcc releases and was not considered an error?

    Yes, the error/warning behavior was changed with version 5.

    As of G++ 5, the behavior is the following: When a later standard is in effect, e.g. when using -std=c++11, narrowing conversions are diagnosed by default, as required by the standard. A narrowing conversion from a constant produces an error, and a narrowing conversion from a non-constant produces a warning, but -Wno-narrowing suppresses the diagnostic.

    I would've expected v4.8.1 to throw a warning at least, but maybe that has been ignored.

    Because of the use of sizeof in the snippet below to control the for loop terminal count, I think the 16 bit size was supposed to be the data type per element of the array. Is that accurate?

    Yes, this further supports that the array type should've been uint16 in the first place.

    If the size of the element was 16 bits, then is the correct fix simply to make that array of type int rather than char?

    Yes.