WARNING: OOInteger broken on Linux GNUStep-1.16

[Micha]

Hi,

In newish Linux distributions (eg. Ubuntu 8.10) with GNUStep 1.16.1 (possibly other minor release of GNUStep as well), OOInteger is actually typecast to an unsigned long instead of to a long.


Funnily enough, this breaks many bits of code within Oolite in rather interesting ways.


There are 2 ways to fix it:

1) System-wide:
Edit /usr/include/GNUstep/GNUstepBase/GSConfig.h:134
- typedef gsuaddr gsaddr;
+ typedef gssaddr gsaddr;

2) Oolite-only:
Edit src/Core/OOCocoa.h
- typedef NSInteger OOInteger;
- typedef NSUInteger OOUInteger;
+ typedef long OOInteger;
+ typedef unsigned long OOInteger;


Now -that- was fun

- Micha.


PS. For those interested, my bug-hunting started with the fact that exiting traders were always blocked from jumping out and ended up cruising around in big loops behind the station.

This was due to the fact that the Coriolis was almost an order-of-magnitude more massive under Linux than under OS-X. This ended up being due to the abovementioned bug causing a low-level loop not to sub-divide octrees properly.

So this bug went from the highest-level (AI plist) to one of the lowest in the code-base. Pfew!

[Cmdr James]

I dont think it makes sense to make OOInteger a long. It just feels wrong. Sounds like a bug in that version of GNUStep to me, so Id say correct it there...

[Micha]

I was just going with what was already in the GNUStep headers, ie, "NSInteger" being cast to long, not int. On 32-bit it won't make a difference, on 64-bit it may.

My 64-bit main Linux machine is now getting the same results as OS-X in the areas I was looking at.

Hopefully a proper upstream fix will be in place soon.

[Micha]

I just checked and on 32-bit Linux, NSInteger and NSUInteger are cast to unsigned int, on 64-bit Linux it's unsigned long.

The reason behind this difference is that NSInteger is the word-size of the machine whereas the native int may or may not be.

[Cmdr James]

Fair enough.

[Solas]

Is it possible that OOInteger is related to "Appearing then exploding rocks in the distance 1.73"
https://bb.oolite.space/viewtopic.php?t=5702

I'm not sure that rescaling in ShipEntity.m .. [wreck rescaleBy: scale_factor] applies to boulders.

the problem was reported on debian lenny .. might this file be connected \trunk\debian\patches\11_warnings.patch

[_ds_]

the problem was reported on debian lenny .. might this file be connected \trunk\debian\patches\11_warnings.patch

No, no such file.

There is, however, a file named "11_warnings.patch" in "trunk/debian/patches". Is that what you meant?

I've not been applying that patch. Perhaps I should… incidentally, breakage: due to recent SDL sound-related commits, 10_oolite.sound.patch needs to be updated.

[Solas]

Solas wrote: the problem was reported on debian lenny .. might this file be connected \trunk\debian\patches\11_warnings.patch
_ds_ wrote: No, no such file. There is, however, a file named "11_warnings.patch" in "trunk/debian/patches". Is that what you meant?

///


trunk/debian/patches/11_warnings.patch (2 KB, 06/01/2009 16:51:39)
9 -static OOInteger CompareEntitiesByDistance(id a, id b, void *relativeTo);
18 -static OOInteger CompareEntitiesByDistance(id a, id b, void *relativeTo)

trunk/Mac-DebugOXP/Source/OOShipGroupDebugInspectorModule.m (3 KB, 09/02/2009 12:46:14)
59 OOInteger clickedRow = [sender clickedRow];

trunk/src/Core/GameController.m (27 KB, 01/04/2009 13:32:13)
352 static OOInteger CompareDisplayModes(id arg1, id arg2, void *context)

trunk/src/Core/Geometry.h (3 KB, 06/01/2009 16:51:51)
38 OOInteger n_triangles; // how many triangles in the geometry
39 OOInteger max_triangles; // how many triangles are allowed in the geometry before expansion

trunk/src/Core/Geometry.m (20 KB, 06/01/2009 16:51:51)
112 OOInteger i, j;
143 OOInteger i, x, y, z;
163 OOInteger i, j;
310 OOInteger i;
329 OOInteger i;
348 OOInteger i;
478 OOInteger i;
607 OOInteger i;

trunk/src/Core/HeadUpDisplay.m (76 KB, 06/04/2009 23:57:29)
1922 OOInteger x;
1923 OOInteger y;

trunk/src/Core/OOCocoa.h (9 KB, 15/03/2009 17:16:48 )
255 /* OOInteger and OOUInteger: int (32-bit) on 32-bit platforms, long (64-bit)
269 typedef NSInteger OOInteger;
277 typedef int OOInteger;
283 typedef NSInteger OOInteger;
287 typedef int OOInteger;
322 typedef OOInteger OOComparisonResult;

trunk/src/Core/OOCollectionExtractors.h (19 KB, 06/01/2009 16:51:51)
81 - (OOInteger)integerAtIndex:(OOUInteger)index defaultValue:(OOInteger)value;
120 - (OOInteger)integerAtIndex:(OOUInteger)index;
167 - (OOInteger)integerForKey:(id)key defaultValue:(OOInteger)value;
206 - (OOInteger)integerForKey:(id)key;
479 OOINLINE OOInteger OOIntegerFromObject(id object, OOInteger defaultValue)
484 OOINLINE OOInteger OOUIntegerFromObject(id object, OOUInteger defaultValue)
489 OOINLINE OOInteger OOIntegerFromObject(id object, OOInteger defaultValue)
494 OOINLINE OOInteger OOUIntegerFromObject(id object, OOUInteger defaultValue)

trunk/src/Core/OOCollectionExtractors.m (35 KB, 06/01/2009 16:51:51)
91 - (OOInteger)integerAtIndex:(OOUInteger)index defaultValue:(OOInteger)value
93 return OOIntegerFromObject([self objectAtIndex:index], value);
269 - (OOInteger)integerAtIndex:(OOUInteger)index
433 - (OOInteger)integerForKey:(id)key defaultValue:(OOInteger)value
435 return OOIntegerFromObject([self objectForKey:key], value);
611 - (OOInteger)integerForKey:(id)key

trunk/src/Core/Entities/PlayerEntityControls.m (82 KB, 06/04/2009 23:57:29)
1927 OOInteger displayModeIndex = [controller indexOfCurrentDisplayMode];

trunk/src/Core/Scripting/OOJSSystem.m (35 KB, 01/04/2009 13:32:13)
53 static OOInteger CompareEntitiesByDistance(id a, id b, void *relativeTo);
1002 static OOInteger CompareEntitiesByDistance(id a, id b, void *relativeTo)

[Micha]

The 11_warnings patch quite accidentally actually fixes this problem in those 2 functions which it patches the return-type on - the patch was made just to remove a compile-time-warning. More investigation at the time would have highlighted the issue earlier..

OOInteger in general will be fine, except anywhere where it can also be negative, such as in that octree loop which I found.

I merely mentioned it here since I've had lots of distance-related niggles, all of which boiled down to that one loop. There may well be other affects which I haven't noticed so I thought I'd give the heads-up to any other developers working on Linux.

I find it interesting that such a big bug could creep into stable versions of Debian.

FWIW, the current Windows-based build system using the 0.19.2 GNUStep packages is fine.

[another_commander]

FWIW, the current Windows-based build system using the 0.19.2 GNUStep packages is fine.

Are you sure of that Micha? In my GNUstep tree, I see this in GSConfig.h:

Code: Select all

/*
 * Integer type with same size as a pointer
 */
typedef	unsigned int gsuaddr;
typedef	int gssaddr;
typedef	gsuaddr gsaddr;

and in NSObjCRuntime.h, I see

Code: Select all

typedef	gsaddr	NSInteger;
typedef	gsuaddr	NSUInteger;

which means that NSInteger in the gnustep-base-1_15.dll distributed with Oolite is buggy, even if it may not show immediately. In any case, I have one fresh gnustep-base-1_15.dll with the fix applied already built, so it won't be too much of a hassle commiting the fix.

[Micha]

Doh!, Sorry, you're correct, a_c, was obviously still too early for me! However, it doesn't matter since 0.19.2 uses GNUStep 1.5, which is hard-coded in Oolite to use int/unsigned int for OOInteger. Oolite only uses the GNUStep-defined NSInteger from GNUStep 1.16 onwards:

Code: Select all

#elif OOLITE_GNUSTEP
	#if GNUSTEP_BASE_MAJOR_VERSION > 1 || GNUSTEP_BASE_MINOR_VERSION >= 16
		typedef NSInteger OOInteger;
		typedef NSUInteger OOUInteger;
	#else
		// Older versions of GNUstep used int on all systems.
		typedef int				OOInteger;
		typedef unsigned int	OOUInteger;
	#endif
	typedef float			OOCGFloat;
#endif

However, as it stands, this means 64-bit Windows-builds may have issues as well... at least, it'll be different to 64-bit OS-X and Linux. Windows uses LLP64 (int and long stay 32-bit) which means 64-bit Windows has a 32-bit OOInteger. But at least it'll be correctly signed/unsigned

[Solas]

I don,t think there will be any 64-bit Windows-builds soon GNUstep on 64 bit Windows - Patch for gnustep base submitted
http://www.nabble.com/GNUstep-on-64-bit ... 40324.html
I after reading that I gave up looking. another_commander was most helpful in bringing Windows-build instructions up to date