Using sizeof with global structure

[RichG]

Question for all...

I'm trying to determine the size of a global structure. It has to be declared globally because it's used in several different files. When compiling (BCC32) the code I get the error:

Error E2109 MainTest.c 9: Not an allowed type in function main

I know this is happening because the array of structures is of an undefined size []. But I can't define the size ahead of time.

The code was compiled from the command line with the command:

BCC32 Test.c MainTest.c

If there is a better way to do this I'm open to suggestions.

Here's a small version of the code:

MainTest.c:

#include "test.h"
#include "stdlib.h"
#include "stdio.h"

void main(void)
{
  int i;
  
  i = sizeof(TextStruct)/sizeof(*TextStruct);
  printf("Size = %d\n", i);
}

Test.h:

typedef struct
{
  char *text;
  int j;
} TextStructDef;

extern TextStructDef TextStruct[];

Test.c:

#include "test.h"

TextStructDef TextStruct[]=
{
 {"aaa", 1},
 {"bbb", 2},
 {"ccc", 3},
};

[Banfa]

You could write a function in Test.c that returns the size of your array using the formula you already have.

That formula should work in Test.c because the size of the array is defined there.

Obviously you would declare the function in Test.h.

[Banfa]

P.S. main should be declared

int main()

not

void main()

The C (and C++) standard specifies that main returns int, not returning int is at best using a compiler extension making your code unnecessarily non-portable and at worst undefined behaviour resulting in anything happening.

[donbock]

You must have a file someplace where the TextStruct array is defined (not merely declared). In that same file you can define a global constant:

#include "test.h"
TextStructDef TextStruct[100];
const int TextStructArraySize = (int) (sizeof(TextStruct) / sizeof(TextStruct[0]));

Add a declaration for TextStructArray Size to test.h.

[RichG]

Thanks guys! Those both will work... either adding a function in Test.c which returns the size, or declaring a global constant below the array definition (though I can't use [100] because the number of elements will change-- not dynamically where I would need to allocate space, but in the source file)

I ended up with something like this:

MainTest.c:

#include "test.h"
#include "stdlib.h"
#include "stdio.h"

int main(void)
{
  printf("Size = %d\n", TextStructSize());
  return(0);
}

Test.h:

int TextStructSize(void);

typedef struct
{
  char *text;
  int j;
} TextStructDef;

extern TextStructDef TextStruct[];

Test.c:

#include "test.h"

TextStructDef TextStruct[]=
{
 {"aaa", 1},
 {"bbb", 2},
 {"ccc", 3},
};

int TextStructSize(void)
{
  return sizeof(TextStruct)/sizeof(TextStruct[0]);
}

[weaknessforcats]

Try to avoid doing this. It's bad practice:

#include "test.h" 
  
TextStructDef TextStruct[]= 
{ 
 {"aaa", 1}, 
 {"bbb", 2}, 
 {"ccc", 3}, 
}; 
  
int TextStructSize(void) 
{ 
  return sizeof(TextStruct)/sizeof(TextStruct[0]); 
}

The problems are:
1) the array is on the stack. Often stack is limited and you might crash at run time trying to allocate the array. This is done before main() starts so your crash will occur before you can debug it.
2) using sizeof to detrermine an array size only works of the array is on the local stack. Otherwise sizeof returns the size of the address of the array.
3) your "aaa" etc.. are 4 byte arrays. you have no boundary value code to insure these arrays correct.
4) all code wo manage the TextStructDef array is not in this file. That means you have spaghetti code since the array management is now scattered all over the program.

I would do this:
1) Write a function you call in main() that creates the array wwith the correct number of elments on the heap.
2) write a series of functions wo manage this array
3) throughout your program call only the array management functions. Do not access the array in any way from another file.

[Banfa]

1) the array is on the stack. Often stack is limited and you might crash at run time trying to allocate the array. This is done before main() starts so your crash will occur before you can debug it.

That isn't true. Generally data with the auto storage specifier is put on the stack. That is data allocated inside a function on the whole. This data is not allocated inside a function. Certainly in Windows and on every embedded platform I have used data declared like this is placed inside the DATA segment of the program which also tends to contain both the stack and the heap.

I do agree that nputting large quantities of data on the stack is generally a bad idea.

2) using sizeof to detrermine an array size only works of the array is on the local stack. Otherwise sizeof returns the size of the address of the array.

I don't think that is a very good way of saying it personally. sizeof only returns the size of the array if you are actually passing the array to the sizeof operator. If you are passing a pointer then you get the size of the pointer.

3) your "aaa" etc.. are 4 byte arrays. you have no boundary value code to insure these arrays correct.

In this particular instance it is not required as only the pointer to the string constant is being stored the is no boundary issue. However it might be better to declare the pointers as const char *.

4) all code to manage the TextStructDef array is not in this file. That means you have spaghetti code since the array management is now scattered all over the program.

Strictly speaking you can't say that as there are no other functions or code accessing the array than the one in the file you claim doesn't contain all the handling code in the posted example.

However given that the array is not declared static and therefore has program wide linkage scope that may not be an unreasonable assumption.

1) Write a function you call in main() that creates the array wwith the correct number of elments on the heap.

We have had this argument before :D butI know plenty of cases where you would want to avoid heap allocation if possible. It is only your experience, which is different to others that suggests that heap allocation be preferred.

2) write a series of functions wo manage this array

I whole heartedly agree with this. But I would make the array static rather than allocated from the heap, then you don't risk the possibility of a failed heap allocation.

3) throughout your program call only the array management functions. Do not access the array in any way from another file.

Again I agree with this.

[RichG]

Good points guys! I do appreciate the input.

Going back to my initial post, when I change the include line to be test.c instead of test.h, the program compiles & works. Any negative to that?

This is starting to open a can beyond my original question... in the actual program, there are about 20 arrays like this each with up to 20 to 30 elements. The text can vary in length but is fixed so I can use const char *. The number is a boolean which may change based on configuration.

The structure is a list of menu options presented to a user based on device configuration.. . therefore all choices will be defined in the structure, but only select ones will be shown to the operator.

So far I've not run into any memory allocation issues, but that doesn't mean they won't happen in the future. I'd like to make sure I'm designing the code properly up front as a rewrite of all this would be a HUGE task. I do appreciate the guidance here!

[weaknessforcats]

This is starting to look like a linked list of TextStructDef since you don't know the exact number of elements.

That's why I would allocate one TextStructDef, populate it and then add the address of this thing to a linked list of such addresses. Then I would just work with the list to get at the TextStructDef variables.

[RichG]

The actual structure may look something like this:

TextStructDef TextStruct[]=
{
 {"First option"        , TRUE},
 {"Second choice"       , TRUE},
 {"Choice number three" , TRUE},
 {"Fourth"              , TRUE},
 {"Fifth selection"     , TRUE},
};

When the code is run, assume options are such that items 3 and 5 are hidden. The array then becomes

TextStructDef TextStruct[]=
{
 {"First option"        , TRUE},
 {"Second choice"       , TRUE},
 {"Choice number three" , FALSE},
 {"Fourth"              , TRUE},
 {"Fifth selection"     , FALSE},
};

An old DOS looking menu is then shown to the user which looks like this:

First option
Second choice
Fourth


Any tips on how to set this up with a linked list?

[donbock]

Going back to my initial post, when I change the include line to be test.c instead of test.h, the program compiles & works. Any negative to that?

Are you suggesting there are errors when main.c includes test.h?

Having main.c include test.c has the following disadvantages:

1. There may be legitimate reasons to include a .c file; but you should realize that you will alarm anybody else who reads your source code.
2. Suppose you have some other file (call it foo.c) that wants to access the TextStruct array variable or call the TextStructSize function. If foo.c follows suit and includes test.c then global symbols TextStruct and TextStructSize are defined in both main.c and foo.c. The result is a pair of duplicate definition errors.

[RichG]

Are you suggesting there are errors when main.c includes test.h?

Having main.c include test.c has the following disadvantages:

1. There may be legitimate reasons to include a .c file; but you should realize that you will alarm anybody else who reads your source code.
2. Suppose you have some other file (call it foo.c) that wants to access the TextStruct array variable or call the TextStructSize function. If foo.c follows suit and includes test.c then global symbols TextStruct and TextStructSize are defined in both main.c and foo.c. The result is a pair of duplicate definition errors.

True on the second point... which takes me right back to my initial problem. Forget I even mentioned it. ;)

[donbock]

which takes me right back to my initial problem.

Wait ... I wouldn't expect you to get any errors or warnings from the code you posted here. If you are getting errors or warnings then please post them -- there really is a way to make this work.

[Banfa]

True on the second point... which takes me right back to my initial problem. Forget I even mentioned it. ;)

Surely your original problem was fixed right around post 5 of this thread?

[Banfa]

This is starting to open a can beyond my original question... in the actual program, there are about 20 arrays like this each with up to 20 to 30 elements. The text can vary in length but is fixed so I can use const char *. The number is a boolean which may change based on configuration.

The structure is a list of menu options presented to a user based on device configuration.. . therefore all choices will be defined in the structure, but only select ones will be shown to the operator.

You say device as though this is some embedded application rather than a program running on a PC. The important point here is is memory, RAM, an issue? Is there not much of it, remembering that it is easy to use up too much RAM at the beginning of an embedded project not leaving yourself enough to at the end of development?

If RAM usage is an issue then using an allocated linked list as weaknessforcats has suggested has the problem that you use double the amount of memory for every menu item that is in your menu (or possibly your current menu) because it has to be in memory in order for you to be able to allocate a list node and copy it to the list node.

On the other hand in memory is not an issue using a linked list could well simplify processing the menu rather than running down arrays looking at boolean variables.

There is in fact an in-between solution; you use a linked list but do not allocate the data from the heap. Instead since the data is already allocated in your data segment you just add the required pointers to the structure and have a routine that sets up the linked list in the array of structures by writing the relevant values to the pointers.

struct menu {
      const char *text;
      struct menu *next;
};

static struct menu deviceMenuData[] = 
{
    {"Menu Item 1", NULL},
    {"Menu Item 2", NULL},
    {"Menu Item 3", NULL},
    {"Menu Item 4", NULL},
    {"Menu Item 5", NULL}
};

static struct menu deviceMenuStart = {"", NULL};

void IntialiseDeviceMenu(void)
{
    int ix;
    int size = sizeof deviceMenuData / sizeof deviceMenuData[0];
    struct menu *current = &deviceMenuStart;

    for(ix=0; ix<size; ix=0)
    {
        if (GetMenuItemEnabledFromConfiguration(ix+1) != FALSE)
        {
            current->next = &deviceMenuData[ix];
        }
    }

    current->next = NULL
}