Hey Stefan
On 2011-04-26, at 8:45 PM, Stefan Bidi wrote:
> On Tue, Apr 26, 2011 at 9:17 PM, Eric Wasylishen <[email protected]>
> wrote:
> Hey.
> -[NSNumberFormatter init] has this block of code:
>
> internal->_behavior = _defaultBehavior;
> internal->_locale = RETAIN([NSLocale currentLocale]);
> internal->_style = NSNumberFormatterNoStyle;
> internal->_symbols = NSZoneCalloc (z, MAX_SYMBOLS, sizeof(id));
> internal->_textAttributes = NSZoneCalloc (z, MAX_TEXTATTRIBUTES, sizeof(id));
> internal->_attributes = NSZoneCalloc (z, MAX_ATTRIBUTES, sizeof(int));
>
> which allocates some C arrays. However, these ivars are never initialized if
> you use -initWithCoder:, and so if you call -setDecimalSeparator: on a
> instance loaded from a nib, you'll get a segfault in this line of code in the
> SET_SYMBOL macro:
>
> if (internal->_symbols[key])
>
> since the internal->_symbols will be NULL at that point.
>
> Thanks for spotting that. I seem to have forgotten about the NSCoding
> protocol when I added these new ivars. I see where someone (can't remember
> if it was me or not) added a FIXME tag in -initWithCoder:.
>
> I have a couple of comments about this:
>
> 1. The SET_SYMBOL macro wasted me a significant amount of time because I had
> to expand it by hand to locate the segfault. Is there a reason this isn't a
> private instance method instead of a macro? (same with SET_TEXTATTRIBUTE,
> SET_ATTRIBUTE, SET_BOOL, GET_SYMBOL, GET_ATTRIBUTE, GET_BOOL).
> Sorry to sound negative; the new NSNumberFormatter implementation looks
> really nice otherwise :-)
>
> I don't have a good reason as to why I did it. I originally used methods but
> later changed it to macros.
>
I see. I didn't mean to direct my annoyance at you, but at GDB for not letting
you step line by line though macro definitons :-)
I guess it's probably not worth the effort changing these back to methods - the
bug wasn't in the macro definitions, anyway.
> 2. I've seen this mistake (-initWithCoder not doing everything -init does)
> before, and I expect it occurs elsewhere in GNUstep. Do we have a policy on
> how to prevent this from happening in the future? i.e. what's the right way
> to implement -initWithCoder and -init? I think we should have a standard
> pattern we use everywhere.
>
> I'm OK with this or the private method you mentioned in your other e-mail. I
> can't promise I'll get to it right away, though... I haven't had a lot of
> personal time lately.
>
No worries, I committed a fix already so -initWithCoder: works. (using a
private method like I described). I'd be interested to hear what others think
about adopting an approach similar to this for other classes that implement
NSCoding.
Regards,
Eric
_______________________________________________
Gnustep-dev mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/gnustep-dev
