Patchwork =?UTF-8?Q?libobjc:=20fix=20class=5FaddMethod()=20to=20return=20NO=20if?= =?UTF-8?Q?=20the=20method=20already=20exists=20in=20the=20class?=

login
register
mail settings
Submitter Nicola Pero
Date Dec. 23, 2010, 5:32 a.m.
Message ID <1293082327.57225869@192.168.2.228>
Download mbox | patch
Permalink /patch/76477/
State New
Headers show

Comments

Nicola Pero - Dec. 23, 2010, 5:32 a.m.
This patch fixes an obvious bug in the new Objective-C runtime API implementation.
Testcases both for classes in constructions and for classes already constructed
are included.

While the bug is obvious once you know where to look, it was crashing gnustep-base
in the initial (complicated) +initialize phases and it took time to track down.

With this patch applied (and using the latest gnustep-base that I've patched to
recognize the new ObjC runtime API and use it), I can now use GCC 4.6 to compile
GNUstep Objective-C programs that use all the new Objective-C 2.0 features and which
actually work. :-)

Committed to trunk.

Thanks

Patch

Index: gcc/testsuite/ChangeLog
===================================================================
--- gcc/testsuite/ChangeLog     (revision 168198)
+++ gcc/testsuite/ChangeLog     (working copy)
@@ -1,3 +1,9 @@ 
+2010-12-23  Nicola Pero  <nicola.pero@meta-innovation.com>
+
+       * obj-c.dg/gnu-api-2-class.m: Test that class_addMethod() returns
+       NO if the method is already implemented in the class.
+       * obj-c++.dg/gnu-api-2-class.mm: Same change.
+
 2010-12-22  Sebastian Pop  <sebastian.pop@amd.com>
 
        PR tree-optimization/47019
Index: gcc/testsuite/objc.dg/gnu-api-2-class.m
===================================================================
--- gcc/testsuite/objc.dg/gnu-api-2-class.m     (revision 168198)
+++ gcc/testsuite/objc.dg/gnu-api-2-class.m     (working copy)
@@ -140,6 +140,12 @@  int main(int argc, void **args)
                           method_getTypeEncoding (method2)))
       abort ();
 
+    /* Test that if the method already exists in the class,
+       class_addMethod() returns NO.  */
+    if (class_addMethod (new_class, @selector (variable), method_getImplementation (method2),
+                        method_getTypeEncoding (method2)))
+      abort ();
+
     objc_registerClassPair (new_class);    
 
     /* Now, MySubClass2 is basically the same as MySubClass!  We'll
@@ -152,6 +158,15 @@  int main(int argc, void **args)
       if ([o variable] != o)
        abort ();
     }
+
+    /* Now, try that if you take an existing class and try to add an
+       already existing method, class_addMethod returns NO.  This is
+       subtly different from before, when 'new_class' was still in
+       construction.  Now it's a real class and the libobjc internals
+       differ between the two cases.  */
+    if (class_addMethod (new_class, @selector (variable), method_getImplementation (method2),
+                        method_getTypeEncoding (method2)))
+      abort ();
   }
 
   printf ("Testing class_addProtocol ()...\n");
Index: gcc/testsuite/obj-c++.dg/gnu-api-2-class.mm
===================================================================
--- gcc/testsuite/obj-c++.dg/gnu-api-2-class.mm (revision 168198)
+++ gcc/testsuite/obj-c++.dg/gnu-api-2-class.mm (working copy)
@@ -140,6 +140,12 @@  int main ()
                           method_getTypeEncoding (method2)))
       abort ();
 
+    /* Test that if the method already exists in the class,
+       class_addMethod() returns NO.  */
+    if (class_addMethod (new_class, @selector (variable), method_getImplementation (method2),
+                        method_getTypeEncoding (method2)))
+      abort ();
+
     objc_registerClassPair (new_class);    
 
     /* Now, MySubClass2 is basically the same as MySubClass!  We'll
@@ -152,6 +158,15 @@  int main ()
       if ([o variable] != o)
        abort ();
     }
+
+    /* Now, try that if you take an existing class and try to add an
+       already existing method, class_addMethod returns NO.  This is
+       subtly different from before, when 'new_class' was still in
+       construction.  Now it's a real class and the libobjc internals
+       differ between the two cases.  */
+    if (class_addMethod (new_class, @selector (variable), method_getImplementation (method2),
+                        method_getTypeEncoding (method2)))
+      abort ();
   }
 
   std::cout << "Testing class_addProtocol ()...\n";
Index: libobjc/sendmsg.c
===================================================================
--- libobjc/sendmsg.c   (revision 168198)
+++ libobjc/sendmsg.c   (working copy)
@@ -759,6 +759,45 @@  class_addMethod (Class class_, SEL selector, IMP i
   if (method_name == NULL)
     return NO;
 
+  /* If the method already exists in the class, return NO.  It is fine
+     if the method already exists in the superclass; in that case, we
+     are overriding it.  */
+  if (CLS_IS_IN_CONSTRUCTION (class_))
+    {
+      /* The class only contains a list of methods; they have not been
+        registered yet, ie, the method_name of each of them is still
+        a string, not a selector.  Iterate manually over them to
+        check if we have already added the method.  */
+      struct objc_method_list * method_list = class_->methods;
+      while (method_list)
+       {
+         int i;
+         
+         /* Search the method list.  */
+         for (i = 0; i < method_list->method_count; ++i)
+           {
+             struct objc_method * method = &method_list->method_list[i];
+             
+             if (method->method_name
+                 && strcmp ((char *)method->method_name, method_name) == 0)
+               return NO;
+           }
+         
+         /* The method wasn't found.  Follow the link to the next list of
+            methods.  */
+         method_list = method_list->method_next;
+       }
+      /* The method wasn't found.  It's a new one.  Go ahead and add
+        it.  */
+    }
+  else
+    {
+      /* Do the standard lookup.  This assumes the selectors are
+        mapped.  */
+      if (search_for_method_in_list (class_->methods, selector))
+       return NO;
+    }
+
   method_list = (struct objc_method_list *)objc_calloc (1, sizeof (struct objc_method_list));
   method_list->method_count = 1;

Index: libobjc/ChangeLog
===================================================================
--- libobjc/ChangeLog   (revision 168198)
+++ libobjc/ChangeLog   (working copy)
@@ -1,3 +1,8 @@ 
+2010-12-23  Nicola Pero  <nicola.pero@meta-innovation.com>
+
+       * sendmsg.c (class_addMethod): Return NO if the method already
+       exists in the class.
+
 2010-12-22  Nicola Pero  <nicola.pero@meta-innovation.com>
 
        * init.c (duplicate_classes): New.