X-Authentication-Warning: delorie.com: mail set sender to djgpp-bounces using -f
X-Recipient: djgpp AT delorie DOT com
Message-ID: <5554DF05.7020707@iki.fi>
Date: Thu, 14 May 2015 20:44:37 +0300
From: "Andris Pavenis (andris DOT pavenis AT iki DOT fi)" <djgpp AT delorie DOT com>
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0
MIME-Version: 1.0
To: djgpp AT delorie DOT com
Subject: Re: bad pragma in dir.h? (and our structrure packing)
References: <CAA2C=vCgLHdH3BJxastGzUsJzhiRddytiYwB1MP_aaiiVpC4nA AT mail DOT gmail DOT com>	<83k2wcjt8e DOT fsf AT gnu DOT org>	<CAA2C=vB-YiGkyx5dJpa=hcBh0O4_NiEKh2tKm5OHyNX3vW7HsQ AT mail DOT gmail DOT com>	<83bnhojnwh DOT fsf AT gnu DOT org>	<CAA2C=vAEKFUktuYXGN_eYUEY0JuHQgXR_-q-N8xox=7PHPEqqw AT mail DOT gmail DOT com>	<838ucsjnbl DOT fsf AT gnu DOT org>	<CAA2C=vAK_T9ixd6YpNrd2LYL80Lau-Dbf+T5vo89AdYXS0WYOw AT mail DOT gmail DOT com>	<83vbfvi3t1 DOT fsf AT gnu DOT org> <CAA2C=vAUa61rumTN9fyCDdtdGXzqfzZLdtz4Nw+8gWX-J-x0bg AT mail DOT gmail DOT com>
In-Reply-To: <CAA2C=vAUa61rumTN9fyCDdtdGXzqfzZLdtz4Nw+8gWX-J-x0bg@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Reply-To: djgpp AT delorie DOT com

On 05/14/2015 07:34 PM, Ozkan Sezer (sezeroz AT gmail DOT com) wrote:
> On 5/14/15, Eli Zaretskii (eliz AT gnu DOT org) <djgpp AT delorie DOT com> wrote:
>>> Date: Wed, 13 May 2015 23:27:39 +0300
>>> From: "Ozkan Sezer (sezeroz AT gmail DOT com)" <djgpp AT delorie DOT com>
>>>
>>>> If you (or someone else) can verify that the size of the structure and
>>>> the offsets of each member are the same in both C and C++ programs,
>>>> then I see no reason to keep the attribute on every member.
>>> I did the following as a foo.c and foo.cc:
>>>
>>> #define COMPILE_TIME_ASSERT(name, x) \
>>>    typedef int _djchk_ ## name[(x) * 2 - 1]
>>>
>>> #include "dos.h"
>>> COMPILE_TIME_ASSERT(find_t,sizeof(struct _find_t)==286);
>>>
>>> #include "dir.h"
>>> COMPILE_TIME_ASSERT(ffblk,sizeof(struct ffblk)==290);
>>> COMPILE_TIME_ASSERT(ffblklfn,sizeof(struct ffblklfn)==318);
>>>
>>> #include "coff.h"
>>> COMPILE_TIME_ASSERT(exln,sizeof(struct external_lineno)==6);
>>> COMPILE_TIME_ASSERT(exse,sizeof(struct external_syment)==18);
>>> COMPILE_TIME_ASSERT(exau,sizeof(union external_auxent)==18);
>>> COMPILE_TIME_ASSERT(exrl,sizeof(struct external_reloc)==10);
>>>
>>> Compiled using existing and modified headers for djgpp using gcc/g++
>>> v3.4.6 and 5.1.0, no errors.  I also compiled its coff.h-only parts
>>> for linux (i686 gcc-4.3.0 and clang-3.4.2), osx (ppc gcc-4.0.1 and
>>> x86_64 gcc-4.2.1), mingw (gcc-3.4.5), mingw-w64 (x86_64 gcc-4.5.4):
>>> no errors.
>>>
>>> Diff for the modified headers is attached (1.patch)
>>> Ideas?
>> LGTM, thanks.
>>
> OK, fixed the broken pragma in dir.h as of r1.7.
>
> Moved the packed attributes from members to structures as
> of dir.h r1.8, dos.h r1.16 and coff.h r1.8.
>
>
>>> On 5/13/15, Andris Pavenis (andris DOT pavenis AT iki DOT fi) <djgpp AT delorie DOT com>
>>> wrote:
>>>> I guess it would be a good idea to have small test in build process to
>>>> verify that offsets of
>>>> members are correct for structures intended for DOS calls. So build
>>>> would
>>>> fail if something is
>>>> wrong with member packing
>>>>
>>>> Andris
>>> We can add compile-time asserts like the above to the headers: ideas?
>> I think a test in djtst would be better.
>>
>> Thanks.
>>
> I would feel safer if the compile time asserts reside in the relevant
> headers and checked every time something is compiled against them.
>
> How about the following patch: (also attached as 3a.patch)
>
> coff.h, dos.h, dir.h: conditionalize packed pragma to gcc < 3.
> add compile time assertions to make sure of correct structure packing.
>
> -/* This is for g++ 2.7.2 and below */
> +#if (__GNUC__ < 3) /* This is for g++ 2.7.2 and below */
>   #pragma pack(1)
> +#endif
>
>   struct ffblk {
>     char lfn_magic[6];			/* LFN */
> @@ -54,7 +55,12 @@
>     char               fd_name[14];
>   } __attribute__((packed));
>
> +#if (__GNUC__ < 3)
>   #pragma pack()
> +#endif
> +/* make sure that structure packing is correct */
> +typedef int _DJCHK_FFBLK0[(sizeof(struct ffblk)   ==290)*3 - 1];
> +typedef int _DJCHK_FFBLK1[(sizeof(struct ffblklfn)==318)*3 - 1];

This way also cross-compiler will detect problems with member packing. At least it seems to be OK 
with gcc-5.1.

There are however some doubts after I took a look at STATIC_ASSERT implementation in Boost:

http://www.boost.org/doc/libs/1_58_0/boost/static_assert.hpp

We do not need all details (we're not interested about MSVC and like) but even for GCC there is 
more than 1 variant.

Andris




Looks OK with