"packed record" in wcxhead.pas for 64-bit

[Zoë [Scooter]]

In wcxhead.pas in WCX Writer's Reference.hlp (2.21se) all of the records except tPackDefaultParamStruct are listed as "packed". In 64-bit Delphi that results in tHeaderDataExW having a sizeof() of 5184 bytes. The C++ header doesn't have anything indicating the packing there, so at least under MSVC, it uses natural packing and sizeof() is 5192 bytes. I don't have C++Builder handy to see what it does.

The fix appears to be to remove the "packed" directive so Delphi uses natural packing too, which makes the two sizes match and makes the fields line up. As far as I can tell it doesn't affect the records in 32-bit builds on way or the other.

Can somebody (Christian?) confirm which alignment/size is correct?

[MVV]

Hm, headers really don't contain any pragmas regarding packing, I agree that they should: there are even structs with WORDs in WDX API header so I suppose that this one should use 2-byte packing...

[milo1012]

Are you sure about the 5184?
If I sum all elements of tHeaderDataExW up, this doesn't seem possible.
Since tHeaderDataExW contains WCHARs, which are defined as 2 bytes in the Windows world (and the necessary headers), no matter which language I use, the only variable thing left is the pointer CmtBuf, which will of course be 8 bytes on x64 and 4 bytes on x86. So no matter if Delphi or C++, the absolute MINIMUM sum will be 5192 on x64 and 5188 on x86. No idea how this could be even less.
And simply applying some logic:
When the C/C++ headers are not explicitly aligned or exhibit any preprocessor definitions for packing instructions, all structure member follow directly after an other in x86 default alignment, i.e. there will be no unnecessary gaps (padding, or "packing", as you call it). So the pascal header needs to do the same, because otherwise they wouldn't be compatible, wcx-interface-wise. We're lucky in this case anyway, because all integers following the FileName element are naturally aligned to a mod(4) offset.

"Alignment" is the key word here, and "packing" is closely related but not used by default in the x86 world unless you force it, see for example:
http://stackoverflow.com/questions/4306186/structure-padding-and-packing

BTW, everything after FileAttr in the tHeaderDataExW structure is not used at all by the past and current wcx interface, so the variable pointer width wouldn't even matter.

[MVV]

So no matter if Delphi or C++, the absolute MINIMUM sum will be 5192 on x64 and 5188 on x86. No idea how this could be even less.

I've counted only 5180 used bytes in 32-bit tHeaderDataExW: 5120 bytes in buffers plus 14x 32-bit integers plus 1x 4-byte pointer = 5180 bytes. And there will be 5184 used bytes in 64-bit tHeaderDataExW due to 64-bit pointer.

When the C/C++ headers are not explicitly aligned or exhibit any preprocessor definitions for packing instructions, all structure member follow directly after an other in x86 default alignment, i.e. there will be no unnecessary gaps (padding, or "packing", as you call it).

Alignment is mostly used for optimization by default so 1-byte alignment will be used only if it is specified explicitly. E.g. default packing value in Visual Studio is 8 so the size of 64-bit tHeaderDataExW is 5192 by default because of 4-byte paddings before CmtBuf and after Reserved fields.

BTW, everything after FileAttr in the tHeaderDataExW structure is not used at all by the past and current wcx interface, so the variable pointer width wouldn't even matter.

Perhaps it is the reason why no packing directives were added to SDK headers yet.

[Zoë [Scooter]]

I've counted only 5180 used bytes in 32-bit tHeaderDataExW: 5120 bytes in buffers plus 14x 32-bit integers plus 1x 4-byte pointer = 5180 bytes. And there will be 5184 used bytes in 64-bit tHeaderDataExW due to 64-bit pointer.

Right, sorry, I had my numbers mixed up: Delphi & MSVC 32-bit are both 5180 bytes, Delphi 64-bit is 5184 bytes, MSVC 64-bit is 5192 bytes.

The reason it came up is because Beyond Compare also supports WCX plug-ins and a customer who was trying to implement one added a ZeroMemory call to their ReadHeaderExW function that was trashing other variables on the stack because our structure was shorter than what they were expecting. I just told them to remove that, since we already do it internally and it doesn't look like the other WCX plugins that provide source do it themselves, but I wanted to make sure we were matching the official implementation going forward. Obviously if any plugins did assign the currently unused fields, it would be good that they're doing it at the right spot too

[milo1012]

I've counted only 5180 used bytes in 32-bit

Indeed, I counted additional ints.

Alignment is mostly used for optimization by default so 1-byte alignment will be used only if it is specified explicitly. E.g. default packing value in Visual Studio is 8 so the size of 64-bit tHeaderDataExW is 5192 by default because of 4-byte paddings before CmtBuf and after Reserved fields

Like I said, alignment is the main term here, and padding or packing is just derived from that, with different compilers using different terms. In C/C++ and x86 all things use a default self-alignment unless you force it to use sth. different:
http://www.catb.org/esr/structure-packing/

Perhaps it is the reason why no packing directives were added to SDK headers yet.

Probably, and because there is no really portable way to force a specific alignment, as each compiler might use different preprocessor macros (C/C++) or keywords (Pascal/Delphi) for that. And I don't see a reason to use tight aligment for that structure in the first place, as you'd only use one ReadHeader structure at a time.

So the main point here is actually quite important:

Obviously if any plugins did assign the currently unused fields, it would be good that they're doing it at the right spot too

Indeed, if TC would start using them, the packed keyword for Pascal/Delphi would make it incompatible with C/C++.

[Hacker]

milo1012,

http://www.catb.org/esr/structure-packing/

Respect++ for referencing catb.org.

Roman

[MVV]

In C/C++ and x86 all things use a default self-alignment unless you force it to use sth. different

Though default behaviour is still compiler-dependent, mentioned self-alignment term doesn't mean that there will be no gaps, it means that fields are aligned according to their sizes (chars use 1-byte alignment, wchars - 2-byte one, ints - 4-byte etc), so e.g. 4-byte gap will be inserted before 8-byte pointer if it is placed after an odd number of 4-byte integers.

Anyway, it would be better and more correct to add explicit alignment directives to SDK headers.

[milo1012]

mentioned self-alignment term doesn't mean that there will be no gaps, it means that fields are aligned according to their sizes

I didn't claim that self-alignment means that there will be no gaps. But I already explained in my previous post (and it's described in the mantioned catb article) that the "gaps" are the natural result of alignment, no matter if using default or some custom alignment (with > 1 byte).

it would be better and more correct to add explicit alignment directives to SDK headers

Sure, but how should these look like, especially when it comes to compatibility with the majority of compilers?
Like I said, there is no really portable way for C/C++. Adding for MSVC and GCC compilers is still quite easy, but that's it; for other compilers you're on your own.
A good and foolproof documentation of the interface is more important IMO, as I had a hard time understanding some things in the past, like when it came to things like this.

[MVV]

We can at least add directives for MSVC and gcc, and for Delphi/Lazarus, it will cover most compilers. If one uses another compiler that doesn't support these directives, he will at least see which alignment should be used.

[milo1012]

Something like this should work:

Code: Select all

#ifdef _MSC_VER
#define ALIGNED8 __declspec(align(8))
#elif defined __GNUC__
#define ALIGNED8 __attribute__((aligned (8),packed))
#else
#define ALIGNED8
#endif

typedef ALIGNED8 struct {
	WCHAR ArcName[1024];
	WCHAR FileName[1024];
	int Flags;
	unsigned int PackSize;
	unsigned int PackSizeHigh;
	unsigned int UnpSize;
	unsigned int UnpSizeHigh;
	int HostOS;
	int FileCRC;
	int FileTime;
	int UnpVer;
	int Method;
	int FileAttr;
	char* CmtBuf;
	int CmtBufSize;
	int CmtSize;
	int CmtState;
	char Reserved[1024];
} tHeaderDataExW;

This would align at 8-byte boundaries.
For the mentioned "gap-less" packing you'd use align(1), but honestly, this is really not sth. you should do, especially when it comes to cache line and SSE for modern CPUs, which needs at least 16-byte boundaries.

[MVV]

Usually 1-byte alignment is used for serialization, here we just need to make these headers independent from project settings or explicit alignment settings.