definition of struct pgm_opt_nak_list is not correct for c++ compilers that also define __STDC_VERSION__

[GoogleCodeExporter]

What steps will reproduce the problem?
1. Compile a program that includes package.h with g++ and -pedantic turned on

What is the expected output? What do you see instead?
Obviously, success is expected. Instead you see:
/usr/include/pgm-5.1/pgm/packet.h:223:19: error: ISO C++ forbids zero-size 
array 'opt_sqn' [-Werror=edantic]

Now, without -pedantic on you are likely to have a successful compile but a 
subtle bug.

What version of the product are you using? On what operating system?
I'm using libpgm-5.1.118. This is on SmartOS, but I'm sure I can reproduce it 
on other platforms, including Linux.

Please provide any additional information below.

The problem is right here:

/* 9.3.5.  Option NAK List - OPT_NAK_LIST
 *
 * GNU C allows opt_sqn[0], ISO C89 requireqs opt_sqn[1], ISO C99 permits opt_sqn[]
 */
struct pgm_opt_nak_list {
        uint8_t         opt_reserved;           /* reserved */
#if defined(__STDC_VERSION__) && (__STDC_VERSION__ >= 199901L)
/* C99 flexible array, sizeof() invalid */
        uint32_t        opt_sqn[];              /* requested sequence number [62] */
#elif !defined(__STDC_VERSION__) || defined(__cplusplus)
/* C90 and older */
        uint32_t        opt_sqn[1];
#else
/* GNU C variable-length object */
        uint32_t        opt_sqn[0];
#endif
};

While ISO C99 permits VLA's, it isn't supported by C++, and a lot of C++ 
compilers when compiling for C++. Since this header could and should be 
included by a C++ program this is relevant. The proper logic is to test for C++ 
and for a __STDC_VERSION__ flag (which is set by gcc, even when compiling C++ 
code) before testing for anything else. Revised code looks like:

/* 9.3.5.  Option NAK List - OPT_NAK_LIST                                       

 *                                                                                                    
 * GNU C allows opt_sqn[0], ISO C89 requireqs opt_sqn[1], ISO C99 permits opt_sqn[]                   
 */
struct pgm_opt_nak_list {                                                       

    uint8_t     opt_reserved;       /* reserved */                                                    
#if defined(__cplusplus) || !defined(__STDC_VERSION__)                          

    uint32_t    opt_sqn[1];                                                                           
#elif (__STDC_VERSION__ >= 199901L)                                             

/* C99 flexible array, sizeof() invalid */                                      

    uint32_t    opt_sqn[];      /* requested sequence number [62] */                                  
#else                                                                           

/* GNU C variable-length object */                                              

    uint32_t    opt_sqn[0];                                                                           
#endif                                                                          

};

Original issue reported on code.google.com by cbsm...@gmail.com on 28 Apr 2012 at 4:14

[GoogleCodeExporter]

Attaching fix as a patch.

Original comment by cbsm...@gmail.com on 28 Apr 2012 at 4:21

Attachments:

• libpgm.pgm_opt_nak_list.cpp.patch

[GoogleCodeExporter]

Which GCC version is this?  Sounds like a compiler bug if __STDC_VERSION__ is 
indicating C99 is fully supported.

Original comment by fnjo...@gmail.com on 1 May 2012 at 2:37

[GoogleCodeExporter]

It's gcc-4.6.2 as built for Solaris/SmartOS. Honestly, even if it was 
indicating only C89 compliance, the code would still select uint32_t 
opt_sqn[0];, which is not a good idea. If you detect __cplusplus, which abhors 
0-length types, you really should only go with the int32_t opt_sqn[1].

Original comment by cbsm...@gmail.com on 1 May 2012 at 3:26

[GoogleCodeExporter]

Yup, I'm ending with the same conclusion.

Original comment by fnjo...@gmail.com on 16 May 2012 at 12:51

• Changed state: Fixed