Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

How to correctly fix "zero-sized array in struct/union" warning (C4200) without breaking the code?

I'm integrating some code into my library. It is a complex data structure well optimized for speed, so i'm trying not to modify it too much. The integration process goes well and actually is almost finished (it compiles). One thing is still bothering me. I'm getting the C4200 warning multiple times:

warning C4200: nonstandard extension used : zero-sized array in struct/union
Cannot generate copy-ctor or copy-assignment operator when UDT contains a zero-sized array

The code works but this warning gives me creeps (especially the part with copy-ctor). THe warning appears because of structures declared like this:

#pragma pack( push )
#pragma pack( 1 )
// String
struct MY_TREEDATSTR
{
    BYTE btLen;
    DWORD dwModOff;
    BYTE btPat[0];
};

typedef MY_TREEDATSTR TREEDATSTR;
typedef MY_TREEDATSTR *PTREEDATSTR;

#pragma pack( pop )

Note the btPat[0]. Is there a way how to easily and correctly get rid of this warning without breaking the code and/or having to change too much in it. Notice the #pragma's, have their any significance according to this warning? And why is the structure declared this way anyway? (I mean the btPat thing, not the #pragma's, those i understand).

Note: i saw this similar question, but it really didn't help me.

Update: as I said, the code works and gives correct results. So a copy-constructor or assignment operator is apparently really not needed. And as i look at the code, none of the structures get memcpy-ed.

like image 299
PeterK Avatar asked Jul 28 '10 07:07

PeterK


3 Answers

If this is a MSVC compiler (which is what the warning message tells me), then you can disable this warning using #pragma warning, ie.:

#pragma warning( push )
#pragma warning( disable : 4200 )
struct _TREEDATSTR
{
    BYTE btLen;
    DWORD dwModOff;
    BYTE btPat[0];
};
#pragma warning( pop )

BTW, the message about the copy-constructor is not creepy, but a good thing because it means, that you can't copy instances of _TREEDATSTR without the unknown bytes in btPat: The compiler has no idea how big _TREEDATSTR really is (because of the 0-size array) and therefore refuses to generate a copy constructor. This means, that you can't do this:

_TREEDATSTR x=y;

which shouldn't work anyway.

like image 98
Nordic Mainframe Avatar answered Nov 18 '22 10:11

Nordic Mainframe


I'll assume that you do want this to be compiled in pure C++ mode, and that you don't want just to compile some files in C and some in C++ and later link.

The warning is telling you that the compiler generated copy constructor and assignment will most probably be wrong with your structure. Using zero-sized arrays at the end of a struct is usually a way, in C, of having an array that is decided at runtime, but is illegal in C++, but you can get similar behavior with a size of 1:

struct runtime_array {
   int size;
   char data[1];
};
runtime_array* create( int size ) {
   runtime_array *a = malloc( sizeof(runtime_array) + size ); // [*]
   a->size = size;
   return a;
}
int main() {
   runtime_array *a = create( 10 );
   for ( int i = 0; i < a->size; ++i ) {
      a->data[i] = 0;
   }
   free(a);
}

This type of structures are meant to be allocated dynamically --or with dynamic stack allocation trickery--, and are not usually copied, but if you tried you would get weird results:

int main() {
   runtime_array *a = create(10);
   runtime_array b = *a;          // ouch!!
   free(a);
}

In this example the compiler generated copy constructor would allocate exactly sizeof(runtime_array) bytes in the stack and then copy the first part of the array into b. The problem is that b has a size field saying 10 but has no memory for any element at all.

If you still want to be able to compile this in C, then you must resolve the warning by closing your eyes: silent that specific warning. If you only need C++ compatibility, you can manually disable copy construction and assignment:

struct runtime_array {
   int size;
   char data[1];
private:
   runtime_array( runtime_array const & );            // undefined
   runtime_array& operator=( runtime_array const & ); // undefined
};

By declaring the copy constructor and assignment operator the compiler will not generate one for you (and won´t complain about it not knowing how). By having the two private you will get compile time errors if by mistake you try to use it in code. Since they are never called, they can be left undefined --this is also used to avoid calling it from within a different method of the class, but I assume that there are no other methods.

Since you are refactoring to C++, I would also make the default constructor private and provide a static public inlined method that will take care of the proper allocation of the contents. If you also make the destructor private you can make sure that user code does not try to call delete on your objects:

struct runtime_array {
   int size;
   char data[1];
   static runtime_array* create( int size ) {
      runtime_array* tmp = (runtime_array*)malloc(sizeof(runtime_array)+size);
      tmp->size = size;
      return tmp;
   }
   static void release( runtime_array * a ) {
      free(a);
   }
private:
   runtime_array() {}
   ~runtime_array() {}
   runtime_array( runtime_array const & );            // undefined
   runtime_array& operator=( runtime_array const & ); // undefined
};

This will ensure that user code does not by mistake create your objects in the stack nor will it mix calls to malloc/free with calls to new/delete, since you manage creation and destruction of your objects. None of this changes affects the memory layout of your objects.

[*] The calculation for the size here is a bit off, and will overallocate, probably by as much as sizeof(int) as the size of the object has padding at the end.

like image 36
David Rodríguez - dribeas Avatar answered Nov 18 '22 09:11

David Rodríguez - dribeas


Try changing it to say btPat[1] instead. I think both C++ and C standards dictate that an array cannot have 0 elements. It could cause problems for any code that rely on the size of the _TREEDATSTR struct itself, but usually these sorts of structs are typecast from buffers where (in this case) the first byte of the buffer determines how many bytes are actually in btPat. This kind of approach relies on the fact that there is no bounds checking on C arrays.

like image 3
dreamlax Avatar answered Nov 18 '22 10:11

dreamlax