[Scummvm-devel] START_PACK_STRUCTS default changed -- port breakage

Max Horn max at quendi.de
Sat Jul 22 19:38:00 CEST 2006


Am 22.07.2006 um 18:46 schrieb Torbjörn Andersson:

> Max Horn wrote:
>
>> My next change will be to remove GCC_PACK (assuming my testings for
>> that turn up positive).
>
> Unfortunately, this change causes regressions. I know LordHoto already
> fixed one (the size of the SCUMM engine's SaveInfoSection changed from
> 26 to 28 when the packed attribute was removed, causing new  
> savegames to
> be broken). The only other I'm aware of is that Full Throttle crashes
> when punching your way out of the dumpster at the very start of the
> game. I assume there are other problems as well, which will have to be
> solved on a case-by-case, or should I say struct-by-struct, basis.
>
> My guess - the documentation isn't entirely clear to me - is that the
> #pragma only affects the alignment of the struct members, while the
> attribute packs it to the smallest possible size.

Nope, that wasn't the cause. The problem was caused by the fact that  
GCC (apparently unlike Visual C++ / Metrowerks ?!?) does not  
interpret macros inside #pragma argument. I.e. this works:

  #pragma pack(1)

but this doesn't:

#define FOO pack(1)
#pragma FOO

I committed a fix to SVN. Interestingly, by simply copying the  
affacted code to a test file, and playing around a little bit with  
it, it took me only 5 minutes to discover the cause of the problem  
and fix it. As opposed to guessing, which probably would still have  
us at the same spot as we started :). Ah well.


> Mind you, I was not aware of any problems with this until after the
> change had been made. Removing dependencies on GCC features seems  
> like a
> good thing to me.

I actually run a query on the list some time ago on the struct  
packing feature of compilers. To the best of my knowledge all the  
compilers we use support #pragma pack(1) -- well at least GCC,  
Metrowerks C++ and Visual C++ do. As a matter of fact, if there were  
any that didn't, then they were not able to run ScummVM before my  
change either, I'd think...

> Actually, I wouldn't mind if we didn't use packed structs at all,  
> but I know that's a lot of work.

That would be quite a big task, and would decrease code readability  
(IMHO)... Normally I'd say: We could undertake it, but I see nothing  
to be gained by it at this time. Show me a compiler we want to  
support that does not provide a suitable struct packing pragma, and  
we can discuss it. That's what I *would* say, normally.

However, I do not care enough anymore what is done. I only committed  
a fix now because I broke it in the first place. You guys shall do  
with it whatever you like (revert to GCC_PACK, rewrite everything,  
implement yet another solution) <shrug>


Bye,
Max



More information about the Scummvm-devel mailing list