Why is the array size unknown?

Question

I tried to program this code on an STM8 Controller:

#include "Imagedata.h"
void main(void)
{
    unsigned char *pArray;
    pArray=IMAGE_DATA;

    while(pArray<=(IMAGE_DATA+(sizeof(IMAGE_DATA)/sizeof(pArray))))
     { 
      SPI_SendData(SPI1,*pArray++ );
      }

}

Actually the array is much longer than this but it would take to much space here. The Array is defined in imagedata.c:

#include "imagedata.h"
const unsigned char IMAGE_DATA[]= { 0X00,0X01,0XC8,0X00,0XC8,0X00};

After compiling this code I get the error message: array size unknown. This refers to the line where I put sizeof(IMAGE_DATA). I don't quiet understand what the problem is. Can anyone help?

Welcome to SO. With the code you show, you should get an error that identifier `IMAGE_DATA` is completely unknown. Do you have a declaration in your header? How does that declaration look? Does it include the size? If not, the compiler cannot know as it don't see the initializer lsit from the other C file. — Gerhardh, Nov 24 '22 at 07:09
`sizeof(IMAGE_DATA)/sizeof(pArray)` makes no sense. `sizeof(IMAGE_DATA)/sizeof IMAGE_DATA[0]` is better. — chux - Reinstate Monica, Nov 24 '22 at 07:29
"This refers to the line where I put sizeof(IMAGE_DATA)." No, this refers to `sizeof(pArray)`. Why do you want the size of a pointer? — chux - Reinstate Monica, Nov 24 '22 at 07:32
So there's typo bugs in main and apart from that the problem isn't reproducible. You need to post how `IMAGE_DATA` is declared in imagedata.h. — Lundin, Nov 24 '22 at 07:36
@Gerhardh @Gerhardh Thank you, this helped. I forgot to include the size in my header: `extern const unsigned char IMAGE_DATA[6];` — , Nov 24 '22 at 07:55
You then should also include the size in the definition of the array. — Gerhardh, Nov 24 '22 at 09:18

Lundin · Answer 1 · 2022-11-24T09:19:16.803

There's multiple problems here.

Try to avoid using global variables with extern in the first place. Implementing setter/getter functions in imagedata.h/imagedata.c is likely a better design. Use private encapsulation whenever possible.
In case the array in the header file is declared as extern const unsigned char IMAGE_DATA[]; you will get the mentioned error message. This creates a reference to an array of "incomplete type". You have to type out the size explicitly like IMAGE_DATA[6], meaning that it probably doesn't make sense to write const unsigned char IMAGE_DATA[]= in the .c file either, because you have to count the bytes anyway.
pArray=IMAGE_DATA; is a constraint violation of the C language. You cannot assign a non-const qualified pointer to a const qualified array.
sizeof(pArray) is nonsense since it gives the size of a pointer, not of a pointed-at item. sizeof a character is guaranteed to be 1 anyway, so this is all rather pointless.
while(pArray<= will lead to out of bounds access, use <. Also use for loops whenever possible since tend to be more readable.
Note #include "imagedata.h" vs #include "Imagedata.h". This will only work on host systems where the file path isn't case-sensitive. So you create a needless non-portability issue between Windows and *nix systems. Always name sources files in pure lower case.

Proposed fixes:

// imagedata.h

/* 
   Normally size_t would be used for array size, but since this is STM8 I picked a 
   byte type, under the assumption that size will not be larger than 256.
*/
const char* image_get_data (unsigned char* size);

// imagedata.c
#include "imagedata.h"

static const unsigned char IMAGE_DATA[]= { 0X00,0X01,0XC8,0X00,0XC8,0X00};

const char* image_get_data (unsigned char* size)
{
  *size = (unsigned char) sizeof(IMAGE_DATA);
  return IMAGE_DATA;
}

// main.c
#include "imagedata.h"
void main(void)
{
  const unsigned char *pArray;
  unsigned char size;
  parray = image_get_data(&size);

  for(unsigned char i=0; i<size; i++)
  {
    SPI_SendData(SPI1, pArray[i]);
  }
}

Note: in case you are using some god-awfully old STM8 compiler, which might be a valid scenario, then disassemble and see if this gave decent machine code. A possible micro-optimization might be to sacrifice some of the private encapsulation for speed:

// imagedata.h
static inline const char* image_get_data (unsigned char* size)
{
   static const unsigned char IMAGE_DATA[]= { 0X00,0X01,0XC8,0X00,0XC8,0X00};

  *size = (unsigned char) sizeof(IMAGE_DATA);
  return IMAGE_DATA;
}

hyde · Answer 2 · 2022-11-24T09:02:52.170

Your condition looks wrong, both the end value and comparison. Try this:

const unsigned char *pArray = IMAGE_DATA;
const unsigned char * const pEnd = pArray + sizeof(IMAGE_DATA) / sizeof(*pArray);

while(pArray < pEnd) {
    ...

Note 1: sizeof char is by definition 1, so / sizeof(*pArray) part is a bit redundant. Matter of taste if it makes code clearer or not.

Note 2: It's a common pattern, and explicitly allowed by C, to have "end" pointer which is one past the end of the range. Just be sure to never do *pEnd.

Note 3: IMAGE_DATA must be array for sizeof to work. It must not be a pointer, or an "array" parameter to function (because that is also a pointer, not really array).

Note 4: IMAGE_DATA is const, so you need a pointer to const. As a side note, code in this answer makes pEnd a const pointer to const, because the end pointer itself shouldn't be accidentally changed, but this is also a matter of taste and optional.

So your actual problem is, IMAGE_DATA isn't fully defined array. Either the definition must include initialization:

char foo[]  = { 33, , 34 , 35 }; // compliers counts size 3

Or with explicit size:

char foo[3];

Why is the array size unknown?

2 Answers2