Re: [Gnustep-cvs] r34822 - in /libs/base/trunk: ChangeLog Source/NSData.m

[Richard Frith-Macdonald]

On 27 Feb 2012, at 19:19, Eric Wasylishen wrote:

> 
> On 2012-02-27, at 3:35 AM, Richard Frith-Macdonald wrote:
> 
>> 
>> On 27 Feb 2012, at 08:09, Fred Kiefer wrote:
>> 
>>> On 27.02.2012 03:12, Eric Wasylishen wrote:
>>>> Author: ericwa
>>>> Date: Mon Feb 27 03:12:19 2012
>>>> New Revision: 34822
>>>> 
>>>> URL: http://svn.gna.org/viewcvs/gnustep?rev=34822&view=rev
>>>> Log:
>>>> * Source/NSData.m: Various 64-bit compatibility fixes, mostly changing
>>>> unsgined to NSUInteger.
>>>> 
>>>> Modified:
>>>>   libs/base/trunk/ChangeLog
>>>>   libs/base/trunk/Source/NSData.m
>>> 
>>> If I checked correctly this patch changed the on disk representation of 
>>> NSData for archiving on 64bit systems. You changed the encoding of length 
>>> from unsigned int to NSUInteger and if the underlying archiver doesn't take 
>>> care of this (Richard, is this the case? Then this mail is pointless), this 
>>> will result in a different representation. Normally we have to increase the 
>>> version number for the class to handle such a change and provide different 
>>> decoding paths for the different versions.
>> 
>> NSArchiver/NSUnarchiver stuff in GNUstep should tolerate different sizes of 
>> integers (a feature introduced many years ago to handle systems with 
>> different encodings for different word sizes), so encoding an NSUInteger 
>> instead of an unsigned should be OK.
>> But, this introduced an inconsistency in that it would encode NSData with an 
>> 'unsigned' value (via the special methods in the coder/decoder for handling 
>> NSData), but now encoding must bale data allowing larger data objects (on 
>> 64bit systems).  I therefore modified NSArchiver/NSUnarchive for consistency 
>> ... though I haven't actually tried archiving/unarching data objects that 
>> large.
> 
> Hm, I seem to be getting lots of test failures now (on 64-bit linux).
> 
> Testing basictypes.m...
> Running base/coding/basictypes.m...
> ./obj/basictypes: Uncaught exception NSInternalInconsistencyException, 
> reason: expected array count 27 and got 116002156320
> /usr/local/bin/gnustep-tests: line 300: 22653 Aborted                 $RUN_CMD
> Failed file:     basictypes.m aborted without running all tests!
> 
> Testing decoding.m...
> Running base/coding/decoding.m...
> Passed test:     decoding.m:198 ... decoding current version of class NSArray
> Failed test:     decoding.m:230 ... decoding class NSArray: <NSException: 
> 0x82bc38> NAME:NSInternalInconsistencyException REASON:expected array count 2 
> and got 9395417345 INFO:(null)
> 
> … etc …
> 
> I think the problem is, in -[NSUnarchiver decodeArrayOfObjCType:count:at:], 
> it calls -[NSData deserializeDataAt:ofObjCType:atCursor:context:] which 
> doesn't do the automatic handling of 32-bit vs 64-bit. So if NSUInteger is 
> 64-bits, decodeArrayOfObjCType:count:at: will attempt to read a 64-bit array 
> length value even if the archive only has a 32-bit length value stored there. 
> Not sure what the solution is… seems to me like you would need a new archive 
> format where array lengths are always stored as 64-bits. 
> 
> But my changes in NSData should be safe without the NSArchiver/NSUnarchiver 
> changes, I think?

Yes, I already fixed this to use a new archive format with variable length 
array counts.  Looks like the old archive format has had this bug practically 
forever (fixed, system-dependent size of array counts) ... which is really 
disappointing given than quite an effort was make to ensure that all the 
archiving of actual values was safe across systems with different  word sizes 
etc.