gnustep-dev
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: some unsigned/NSInteger to NSUInteger changes in -gui


From: Sebastian Reitenbach
Subject: Re: some unsigned/NSInteger to NSUInteger changes in -gui
Date: Tue, 10 Apr 2012 19:18:12 +0200
User-agent: SOGoMail 1.3.14

 
On Tuesday, April 10, 2012 18:36 CEST, David Chisnall <address@hidden> wrote: 
 
> Hi Sebastian,
> 
> On 10 Apr 2012, at 17:27, Sebastian Reitenbach wrote:
> 
> > I thought one of the goals is to be more compatible with Coocoa, and there 
> > its an NSUInteger. therefore I decided to follow your first suggestion, 
> > bumping the NSTabView class version, and add a check to the decoder method. 
> > There is already another check for version < 2.
> > The patch below now again makes the broken Apps work for me.
> > 
> > Is that OK, or is there more that would need to be fixed?
> 
> As someone else said, please consider using @encode(int) instead of "i" and 
> @encode(NSUInteger) instead of "l".  The former is a style thing, the later 
> is just wrong - NSUInteger is a typedef and may not always be l (long).  For 
> the current version, I had a macro version somewhere that was defined as:

Ah, there was this additional explanation missing, now I understand the point, 
thanks.


> 
> #define DECODE(x) [aDecoder decodeValueOfObjCType: @encode(__typeof__(x)) at: 
> &(x)]
> 
> You then just write:
> 
> DECODE(_draws_background);
> DECODE(_truncated_label);
> DECODE(_selected_item);
> 
> and so on.
> 
> Your legacy compatibility code is also subtly wrong.  If int is 32 bits and 
> NSInteger is 64 bits then this is broken on big-endian platforms.  You should 
> instead write:

But then, the version that was there before my change, also had this subtle bug 
already, so even if I made it worse, it helped to spot another bug with your 
help ;)

> 
> if (version < 3)
> {
>       int tmp;
>       [aDecoder decodeValueOfObjCType: @encode(int) at: &tmp];
>       _selected_item = tmp;
> }
> else
> ...


did you meant something like this then, without your nice macro:

Index: NSTabView.m
===================================================================
--- NSTabView.m (revision 35052)
+++ NSTabView.m (working copy)
@@ -52,7 +52,7 @@
 {
   if (self == [NSTabView class])
     {
-      [self setVersion: 2];
+      [self setVersion: 3];
 
       [self exposeBinding: NSSelectedIndexBinding];
       [self exposeBinding: NSFontBinding];
@@ -550,7 +550,7 @@
       [aCoder encodeValueOfObjCType: @encode(BOOL) at: &_draws_background];
       [aCoder encodeValueOfObjCType: @encode(BOOL) at: &_truncated_label];
       [aCoder encodeConditionalObject: _delegate];
-      [aCoder encodeValueOfObjCType: "I" at: &_selected_item];
+      [aCoder encodeValueOfObjCType: @encode(NSUInteger) at: &_selected_item];
     }
 }
 
@@ -631,8 +631,17 @@
       [aDecoder decodeValueOfObjCType: @encode(BOOL) at: &_draws_background];
       [aDecoder decodeValueOfObjCType: @encode(BOOL) at: &_truncated_label];
       _delegate = [aDecoder decodeObject];
-      [aDecoder decodeValueOfObjCType: "I" at: &_selected_item];
-      _selected = [_items objectAtIndex: _selected_item];
+      if (version < 3)
+       {
+         int tmp = (int)_selected_item;
+          [aDecoder decodeValueOfObjCType: @encode(int) at: &_selected_item];
+          _selected = [_items objectAtIndex: tmp];
+       }
+      else
+       }
+          [aDecoder decodeValueOfObjCType: @encode(NSUInteger) at: 
&_selected_item];
+          _selected = [_items objectAtIndex: _selected_item];
+       }
     }
   return self;
 }


otherwise I don't get why I should encode tmp, without giving it a useful value.

with regard to the subtle compatibility bug, the  encoder had this before my 
initial change:

[aCoder encodeValueOfObjCType: "i" at: &_selected_item];

Which then also probably was wrong before the same way like the decoder? So 
when it encoded things wrongly, it should also decode things wrongly to get it 
right again?

But maybe I still did not got the whole picture :(

Sebastian


> 
> David
> 
> -- Send from my Jacquard Loom 
 
 
 



reply via email to

[Prev in Thread] Current Thread [Next in Thread]