[Rprotobuf-commits] r480 - in pkg: . R inst/unitTests man src

[noreply at r-forge.r-project.org]

Author: murray
Date: 2012-09-22 01:19:25 +0200 (Sat, 22 Sep 2012)
New Revision: 480

Modified:
   pkg/ChangeLog
   pkg/R/containing_type.R
   pkg/inst/unitTests/runit.FieldDescriptor.R
   pkg/man/containing_type-methods.Rd
   pkg/src/S4_classes.h
Log:
Fix bug causing segfaults when containing_type() is caused because the
return value is never checked and the code attempted to dereference
NULL pointers.  Add documentation and tests showing this working
properly now.



Modified: pkg/ChangeLog
===================================================================
--- pkg/ChangeLog	2012-08-22 06:18:04 UTC (rev 479)
+++ pkg/ChangeLog	2012-09-21 23:19:25 UTC (rev 480)
@@ -1,3 +1,17 @@
+2012-09-21  Murray Stokely  <murray at FreeBSD.org>
+
+	* Fix a bug causing segfaults in containing_type().
+	* R/containing_type.R: Return NULL instead of invalid descriptors
+	when there is no containing type for an object.
+	* src/S4_classes.h: Check for NULL pointers in the contructors for
+	S4_Descriptor and S4_EnumDescriptor as these methods are called on
+	the return value of e.g. containing_type() in the protobuf API
+	without checking the return alue.
+	* man/containing_type-methods.Rd: Add examples section showing
+	cases where there is and is not a containing_type.
+	* inst/unitTests/runit.FieldDescriptor.R
+	(test.FieldDescriptor.class): Add tests for the above.
+
 2012-08-21  Murray Stokely  <murray at FreeBSD.org>
 
 	* src/mutators.cpp: Add better input checking when setting an

Modified: pkg/R/containing_type.R
===================================================================
--- pkg/R/containing_type.R	2012-08-22 06:18:04 UTC (rev 479)
+++ pkg/R/containing_type.R	2012-09-21 23:19:25 UTC (rev 480)
@@ -4,12 +4,26 @@
 } )
 
 setMethod( "containing_type", "Descriptor", function(object){
-	.Call( "Descriptor__containing_type", object at pointer, PACKAGE = "RProtoBuf" )
+	retval <- .Call( "Descriptor__containing_type", object at pointer, PACKAGE = "RProtoBuf" )
+	if (length(retval at type) == 0) {
+		# Descriptors do not always have containing types.
+		# In such cases NULL is better return value than malformed Descriptor.
+		return(NULL)
+	} else {
+                return(retval)
+	}
 } )
 setMethod( "containing_type", "EnumDescriptor", function(object){
-	.Call( "EnumDescriptor__containing_type", object at pointer, PACKAGE = "RProtoBuf" )
+	retval <- .Call( "EnumDescriptor__containing_type", object at pointer, PACKAGE = "RProtoBuf" )
+	if (length(retval at name) == 0) {
+                # If this enum type is nested in a message type, this is that message type.
+                # Otherwise, NULL.
+		return(NULL)
+	} else {
+                return(retval)
+	}
 } )
 setMethod( "containing_type", "FieldDescriptor", function(object){
+        # Never NULL
 	.Call( "FieldDescriptor__containing_type", object at pointer, PACKAGE = "RProtoBuf" )
 } )
-

Modified: pkg/inst/unitTests/runit.FieldDescriptor.R
===================================================================
--- pkg/inst/unitTests/runit.FieldDescriptor.R	2012-08-22 06:18:04 UTC (rev 479)
+++ pkg/inst/unitTests/runit.FieldDescriptor.R	2012-09-21 23:19:25 UTC (rev 480)
@@ -55,4 +55,10 @@
 
   # Return the class of a message field
   checkTrue(inherits(message_type(Person$phone), "Descriptor"))
+
+  # Containing type of a field is the message descriptor
+  checkTrue(inherits(Person$id$containing_type(), "Descriptor"))
+
+  # No containing type for the top-level message descriptor.
+  checkTrue(is.null(Person$containing_type()))
 }

Modified: pkg/man/containing_type-methods.Rd
===================================================================
--- pkg/man/containing_type-methods.Rd	2012-08-22 06:18:04 UTC (rev 479)
+++ pkg/man/containing_type-methods.Rd	2012-09-21 23:19:25 UTC (rev 480)
@@ -11,5 +11,11 @@
 \linkS4class{Descriptor}, \linkS4class{EnumDescriptor}, 
 \linkS4class{FieldDescriptor}
 }
+\examples{
+# Containing type of a field is the message descriptor
+tutorial.Person$id$containing_type()
+
+# No containing type for the top-level message descriptor.
+tutorial.Person$containing_type()
+}
 \keyword{methods}
-

Modified: pkg/src/S4_classes.h
===================================================================
--- pkg/src/S4_classes.h	2012-08-22 06:18:04 UTC (rev 479)
+++ pkg/src/S4_classes.h	2012-09-21 23:19:25 UTC (rev 480)
@@ -54,7 +54,11 @@
 		S4_Descriptor( const GPB::Descriptor* d) : S4( "Descriptor" ){
 			slot( "pointer" ) = Rcpp::XPtr<GPB::Descriptor>( 
 				const_cast<GPB::Descriptor*>(d), false) ;
-			slot( "type" )    = d->full_name() ;
+			if (!d) {
+				slot( "type" ) = Rcpp::StringVector(0) ;
+			} else {
+				slot( "type" ) = d->full_name() ;
+			}
 		}
 		
 		S4_Descriptor( const S4_Descriptor& other) : S4(){
@@ -141,15 +145,18 @@
 		S4_EnumDescriptor( const GPB::EnumDescriptor* d) : S4( "EnumDescriptor" ){
 			slot( "pointer" ) = Rcpp::XPtr<GPB::EnumDescriptor>( 
 				const_cast<GPB::EnumDescriptor*>(d), false) ;
-			slot( "name" )     = d->name() ;
-			slot( "full_name") = d->full_name() ;
-			const GPB::Descriptor *type_desc = d->containing_type() ;
-			if( type_desc ){
-				slot( "type" ) = type_desc->full_name()  ;
-			} else{
-				slot( "type" ) = Rcpp::StringVector(0) ;
+			slot( "type" ) = Rcpp::StringVector(0) ;
+			if (d) {
+				slot( "name" )	   = d->name() ;
+				slot( "full_name") = d->full_name() ;
+				const GPB::Descriptor *type_desc = d->containing_type() ;
+				if( type_desc ){
+					slot( "type" ) = type_desc->full_name()	 ;
+				}
+			} else {
+				slot( "name" )	   = Rcpp::StringVector(0) ;
+				slot( "full_name") = Rcpp::StringVector(0) ;
 			}
-			
 		}
 		
 		S4_EnumDescriptor( const S4_EnumDescriptor& other) : S4(){