vipsobject cleanups

vipsobject always goes via set_prop so it can work with subclasses
which override these funcs (eg. wrap7)

TODO

@@ -1,3 +1,21 @@
+- vips_wrap7_object_set_property() needs to do the thing with reffing the
+  object or the operation, depending on the param direction
+
+  also watch close signal
+
+- vips_object_free_argument() uses offset directly to free strings, disaster
+  with wrap7
+
+  instead, it should use g_object_set() to set the vars to NULL, that way
+  it'll work for wrap7 as well
+
+  check for other uses of offset
+
+  also vips_object_dispose_argument()
+
+
+
+
 
 - revisit orc conv
 

libvips/deprecated/wrapvips7.c

@@ -382,14 +382,9 @@ vips_wrap7_object_set_property( GObject *gobject,
 
 	case VIPS_WRAP7_IMAGE:
 	case VIPS_WRAP7_INTERPOLATE:
-		/* This does not add a ref to object.
+		vips__object_set_member( object, pspec,
-		 */
+			(GObject **) &wrap7->vargv[i], 
-		wrap7->vargv[i] = g_value_get_object( value );
+			g_value_get_object( value ) );
-
-		/* Now ref the object this operation refs. Drop this ref in
-		 * _dispose(), see above.
-		 */
-		g_object_ref( g_value_get_object( value ) );
 		break;
 
 	default:
@@ -464,6 +459,66 @@ vips_wrap7_object_get_property( GObject *gobject,
 	}
 }
 
+/* Init an output slot in vargv.
+ */
+static void *
+vips_wrap7_build_output( VipsObject *object,
+	GParamSpec *pspec,
+	VipsArgumentClass *argument_class,
+	VipsArgumentInstance *argument_instance,
+	void *a, void *b )
+{
+	VipsWrap7 *wrap7 = VIPS_WRAP7( object );  
+	VipsWrap7Class *class = VIPS_WRAP7_GET_CLASS( wrap7 );
+	int i = argument_class->offset;
+	im_arg_desc *arg = &class->fn->argv[i];
+	im_type_desc *type = arg->desc;
+	im_arg_type vt = type->type;
+
+	/* We want required, construct-time, unassigned output args.
+	 */
+	if( !(argument_class->flags & VIPS_ARGUMENT_REQUIRED) ||
+		!(argument_class->flags & VIPS_ARGUMENT_CONSTRUCT) ||
+		argument_instance->assigned ||
+		!(argument_class->flags & VIPS_ARGUMENT_OUTPUT) )
+		return( NULL ); 
+
+	/* Provide output objects for the operation to write to.
+	 */
+	switch( vips_wrap7_lookup_type( vt ) ) {
+	case VIPS_WRAP7_DOUBLE:
+	case VIPS_WRAP7_INT:
+	case VIPS_WRAP7_COMPLEX:
+	case VIPS_WRAP7_STRING:
+		break;
+
+	case VIPS_WRAP7_IMAGE:
+		g_object_set( object, arg->name, vips_image_new(), NULL ); 
+		break;
+
+	case VIPS_WRAP7_DMASK:
+	case VIPS_WRAP7_IMASK:
+		break;
+
+	case VIPS_WRAP7_GVALUE:
+	{
+		GValue *value = wrap7->vargv[i];
+
+		memset( value, 0, sizeof( GValue ) );
+
+		break;
+	}
+
+	case VIPS_WRAP7_DOUBLEVEC:
+	case VIPS_WRAP7_INTVEC:
+	default:
+		wrap7->error = TRUE;
+		break;
+	}
+
+	return( NULL );
+}
+
 static int
 vips_wrap7_build( VipsObject *object )
 {
@@ -484,6 +539,12 @@ vips_wrap7_build( VipsObject *object )
 		return( -1 );
 	}
 
+	/* Init all the output args.
+	 */
+	(void) vips_argument_map( VIPS_OBJECT( wrap7 ),
+		vips_wrap7_build_output, 
+		NULL, NULL ); 
+
 	if( VIPS_OBJECT_CLASS( vips_wrap7_parent_class )->build( object ) )
 		return( -1 );
 
@@ -500,9 +561,9 @@ vips_wrap7_print_class( VipsObjectClass *oclass, VipsBuf *buf )
 	im_function *fn = class->fn;
 
 	if( fn )
-		vips_buf_appendf( buf, "%s, ", fn->name );
+		vips_buf_appendf( buf, "%s ", fn->name );
 	else
-		vips_buf_appendf( buf, "%s, ", G_OBJECT_CLASS_NAME( class ) );
+		vips_buf_appendf( buf, "%s ", G_OBJECT_CLASS_NAME( class ) );
 
 	if( oclass->nickname )
 		vips_buf_appendf( buf, "(%s), ", oclass->nickname );
@@ -708,103 +769,6 @@ vips_wrap7_subclass_class_init( VipsWrap7Class *class )
 	}
 }
 
-static void
-vips_wrap7_arg_close( GObject *argument,
-	VipsArgumentInstance *argument_instance )
-{
-	VipsObject *object = argument_instance->object;
-
-	g_object_unref( object );
-}
-
-/* Init an output slot in vargv.
- */
-static void *
-vips_wrap7_build_output( VipsObject *object,
-	GParamSpec *pspec,
-	VipsArgumentClass *argument_class,
-	VipsArgumentInstance *argument_instance,
-	void *a, void *b )
-{
-	VipsWrap7 *wrap7 = VIPS_WRAP7( object );  
-	VipsWrap7Class *class = VIPS_WRAP7_GET_CLASS( wrap7 );
-	int i = argument_class->offset;
-	im_arg_desc *arg = &class->fn->argv[i];
-	im_type_desc *type = arg->desc;
-	im_arg_type vt = type->type;
-
-	/* We want required, construct-time, unassigned output args.
-	 */
-	if( !(argument_class->flags & VIPS_ARGUMENT_REQUIRED) ||
-		!(argument_class->flags & VIPS_ARGUMENT_CONSTRUCT) ||
-		argument_instance->assigned ||
-		!(argument_class->flags & VIPS_ARGUMENT_OUTPUT) )
-		return( NULL ); 
-
-	/* Provide output objects for the operation to write to.
-	 */
-	switch( vips_wrap7_lookup_type( vt ) ) {
-	case VIPS_WRAP7_DOUBLE:
-	case VIPS_WRAP7_INT:
-	case VIPS_WRAP7_COMPLEX:
-	case VIPS_WRAP7_STRING:
-		break;
-
-	case VIPS_WRAP7_IMAGE:
-		/* Output objects ref this operation.
-		 */
-		wrap7->vargv[i] = vips_image_new(); 
-		g_object_ref( wrap7 );
-
-		/* vipsobject will handle close_id disconnect for us.
-		 */
-		argument_instance->close_id =
-			g_signal_connect( wrap7->vargv[i], "close",
-				G_CALLBACK( vips_wrap7_arg_close ),
-				argument_instance );
-		break;
-
-	case VIPS_WRAP7_DMASK:
-	case VIPS_WRAP7_IMASK:
-	{
-		im_mask_object *mo = wrap7->vargv[i];
-
-		mo->mask = NULL;
-		mo->name = im_strdup( NULL, "" );
-
-		break;
-	}
-
-	case VIPS_WRAP7_GVALUE:
-	{
-		GValue *value = wrap7->vargv[i];
-
-		memset( value, 0, sizeof( GValue ) );
-
-		break;
-	}
-
-	case VIPS_WRAP7_DOUBLEVEC:
-	case VIPS_WRAP7_INTVEC:
-	{
-		/* intvec is also int + pointer.
-		 */
-		im_doublevec_object *dv = wrap7->vargv[i];
-
-		dv->n = 0;
-		dv->vec = NULL;
-
-		break;
-	}
-
-	default:
-		wrap7->error = TRUE;
-		break;
-	}
-
-	return( NULL );
-}
-
 static void
 vips_wrap7_subclass_init( VipsWrap7 *wrap7 )
 {
@@ -816,12 +780,6 @@ vips_wrap7_subclass_init( VipsWrap7 *wrap7 )
 		wrap7->error = TRUE;
 		return;
 	}
-
-	/* Init all the output args.
-	 */
-	(void) vips_argument_map( VIPS_OBJECT( wrap7 ),
-		vips_wrap7_build_output, 
-		NULL, NULL ); 
 }
 
 static GType

libvips/include/vips/object.h

@@ -156,6 +156,8 @@ VipsArgumentInstance *vips__argument_get_instance( VipsArgumentClass *,
 	VipsObject *);
 VipsArgument *vips__argument_table_lookup( VipsArgumentTable *, 
 	GParamSpec *);
+void vips__object_set_member( VipsObject *object, GParamSpec *pspec,
+	GObject **member, GObject *argument );
 typedef void *(*VipsArgumentMapFn)( VipsObject *, GParamSpec *,
 	VipsArgumentClass *, VipsArgumentInstance *, void *a, void *b );
 void *vips_argument_map( VipsObject *object, 

libvips/iofuncs/object.c

@@ -333,20 +333,19 @@ vips__argument_get_instance( VipsArgumentClass *argument_class,
 }
 
 static void
-vips_object_clear_object( VipsObject *object, GParamSpec *pspec )
+vips_object_clear_member( VipsObject *object, GParamSpec *pspec, 
+	GObject **member )
 {
 	VipsObjectClass *class = VIPS_OBJECT_GET_CLASS( object );
 	VipsArgumentClass *argument_class = (VipsArgumentClass *)
 		vips__argument_table_lookup( class->argument_table, pspec );
 	VipsArgumentInstance *argument_instance =
 		vips__argument_get_instance( argument_class, object );
-	GObject **member = &G_STRUCT_MEMBER( GObject *, object,
-		argument_class->offset );
 
 	if( *member ) {
 		if( argument_class->flags & VIPS_ARGUMENT_INPUT ) {
 #ifdef DEBUG_REF
-			printf( "vips_object_clear_object: vips object: " );
+			printf( "vips_object_clear_member: vips object: " );
 			vips_object_print_name( object );
 			printf( "  no longer refers to gobject %s (%p)\n",
 				G_OBJECT_TYPE_NAME( *member ), *member );
@@ -360,7 +359,7 @@ vips_object_clear_object( VipsObject *object, GParamSpec *pspec )
 		}
 		else if( argument_class->flags & VIPS_ARGUMENT_OUTPUT ) {
 #ifdef DEBUG_REF
-			printf( "vips_object_clear_object: gobject %s (%p)\n",
+			printf( "vips_object_clear_member: gobject %s (%p)\n",
 				G_OBJECT_TYPE_NAME( *member ), *member );
 			printf( "  no longer refers to vips object: " );
 			vips_object_print_name( object );
@@ -378,7 +377,6 @@ vips_object_clear_object( VipsObject *object, GParamSpec *pspec )
 				g_signal_handler_disconnect( object,
 					argument_instance->close_id );
 			argument_instance->close_id = 0;
-			*member = NULL;
 
 			g_object_unref( object );
 		}
@@ -387,6 +385,19 @@ vips_object_clear_object( VipsObject *object, GParamSpec *pspec )
 	}
 }
 
+/* Clear a member variable, where the member has an offset.
+ */
+static void
+vips_object_clear_object( VipsObject *object, GParamSpec *pspec )
+{
+	VipsObjectClass *class = VIPS_OBJECT_GET_CLASS( object );
+	VipsArgumentClass *argument_class = (VipsArgumentClass *)
+		vips__argument_table_lookup( class->argument_table, pspec );
+
+	vips_object_clear_member( object, pspec, 
+		&G_STRUCT_MEMBER( GObject *, object, argument_class->offset ) );
+}
+
 /* Free any args which are holding resources.
  */
 static void *
@@ -398,23 +409,17 @@ vips_object_dispose_argument( VipsObject *object, GParamSpec *pspec,
 	g_assert( ((VipsArgument *) argument_class)->pspec == pspec );
 	g_assert( ((VipsArgument *) argument_instance)->pspec == pspec );
 
-	if( G_IS_PARAM_SPEC_OBJECT( pspec ) )
+	if( G_IS_PARAM_SPEC_OBJECT( pspec ) ||
-		vips_object_clear_object( object, pspec );
+		G_IS_PARAM_SPEC_BOXED( pspec ) ) {
-	else if( G_IS_PARAM_SPEC_BOXED( pspec ) ) {
-		gpointer *member = &G_STRUCT_MEMBER( gpointer, object,
-			argument_class->offset );
-
 #ifdef DEBUG
 		printf( "vips_object_dispose_argument: " );
 		vips_object_print_name( object );
 		printf( ".%s\n", g_param_spec_get_name( pspec ) ); 
 #endif /*DEBUG*/
 
-		if( *member ) {
+		g_object_set( object, 
-			g_boxed_free( G_PARAM_SPEC_VALUE_TYPE( pspec ),
+			g_param_spec_get_name( pspec ), NULL, 
-				*member );
+			NULL );
-			*member = NULL;
-		}
 	}
 
 	return( NULL );
@@ -446,16 +451,15 @@ vips_object_free_argument( VipsObject *object, GParamSpec *pspec,
 	g_assert( ((VipsArgument *) argument_instance)->pspec == pspec );
 
 	if( G_IS_PARAM_SPEC_STRING( pspec ) ) {
-		char **member = &G_STRUCT_MEMBER( char *, object,
-			argument_class->offset );
-
 #ifdef DEBUG
 		printf( "vips_object_free_argument: " );
 		vips_object_print_name( object );
 		printf( ".%s\n", g_param_spec_get_name( pspec ) ); 
 #endif /*DEBUG*/
 
-		VIPS_FREE( *member );
+		g_object_set( object, 
+			g_param_spec_get_name( pspec ), NULL, 
+			NULL );
 	}
 
 	return( NULL );
@@ -503,15 +507,16 @@ vips_object_dispose( GObject *gobject )
 	/* Clear all our arguments: they may be holding refs we should drop.
 	 */
 	vips_argument_dispose_all( object );
-	VIPS_FREEF( vips_argument_table_destroy, object->argument_table );
 
 	vips_object_close( object );
 
-	G_OBJECT_CLASS( vips_object_parent_class )->dispose( gobject );
-
 	vips_object_postclose( object );
 
 	vips_argument_free_all( object );
+
+	VIPS_FREEF( vips_argument_table_destroy, object->argument_table );
+
+	G_OBJECT_CLASS( vips_object_parent_class )->dispose( gobject );
 }
 
 static void
@@ -548,21 +553,24 @@ vips_object_arg_close( GObject *argument,
 	vips_object_clear_object( object, pspec );
 }
 
-static void
+/* Set a member to an object. Handle the ref counts and signal
-vips_object_set_object( VipsObject *object, GParamSpec *pspec,
+ * connect/disconnect.
-	GObject *argument )
+ */
+void
+vips__object_set_member( VipsObject *object, GParamSpec *pspec,
+	GObject **member, GObject *argument )
 {
 	VipsObjectClass *class = VIPS_OBJECT_GET_CLASS( object );
 	VipsArgumentClass *argument_class = (VipsArgumentClass *)
 		vips__argument_table_lookup( class->argument_table, pspec );
 	VipsArgumentInstance *argument_instance =
 		vips__argument_get_instance( argument_class, object );
-	GObject **member = &G_STRUCT_MEMBER( GObject *, object,
-		argument_class->offset );
 
 	g_assert( argument_instance );
-	g_assert( !*member );
 
+	vips_object_clear_member( object, pspec, member );
+
+	g_assert( !*member );
 	*member = argument;
 
 	if( *member ) {
@@ -593,6 +601,7 @@ vips_object_set_object( VipsObject *object, GParamSpec *pspec,
 			/* The argument reffs us.
 			 */
 			g_object_ref( object );
+			g_assert( !argument_instance->close_id );
 			argument_instance->close_id =
 				g_signal_connect( *member, "close",
 					G_CALLBACK( vips_object_arg_close ),
@@ -601,6 +610,40 @@ vips_object_set_object( VipsObject *object, GParamSpec *pspec,
 	}
 }
 
+static void
+vips_object_set_object( VipsObject *object, GParamSpec *pspec,
+	GObject *argument )
+{
+	VipsObjectClass *class = VIPS_OBJECT_GET_CLASS( object );
+	VipsArgumentClass *argument_class = (VipsArgumentClass *)
+		vips__argument_table_lookup( class->argument_table, pspec );
+	GObject **member = &G_STRUCT_MEMBER( GObject *, object,
+		argument_class->offset );
+
+	vips__object_set_member( object, pspec, member, argument );
+}
+
+/* Is a value NULL? We allow multiple sets of NULL so props can be cleared.
+ */
+static gboolean
+vips_pspec_value_is_null( GParamSpec *pspec, const GValue *value )
+{
+	if( G_IS_PARAM_SPEC_STRING( pspec ) && 
+		!g_value_get_string( value ) )
+		return( TRUE );
+	if( G_IS_PARAM_SPEC_OBJECT( pspec ) &&
+		!g_value_get_object( value ) )
+		return( TRUE );
+	if( G_IS_PARAM_SPEC_POINTER( pspec ) &&
+		!g_value_get_pointer( value ) )
+		return( TRUE );
+	if( G_IS_PARAM_SPEC_BOXED( pspec ) &&
+		!g_value_get_boxed( value ) )
+		return( TRUE );
+
+	return( FALSE );
+}
+
 /* Also used by subclasses, so not static.
  */
 void
@@ -641,7 +684,8 @@ vips_object_set_property( GObject *gobject,
 	 * built.
 	 */
 	if( argument_class->flags & VIPS_ARGUMENT_CONSTRUCT &&
-		object->constructed ) {
+		object->constructed &&
+		!vips_pspec_value_is_null( pspec, value ) ) {
 		g_warning( "%s: %s can't assign '%s' after construct",
 			G_STRLOC,
 			G_OBJECT_TYPE_NAME( gobject ),
@@ -652,7 +696,8 @@ vips_object_set_property( GObject *gobject,
 	/* If this is a set-once argument, check we've not set it before.
 	 */
 	if( argument_class->flags & VIPS_ARGUMENT_SET_ONCE &&
-		argument_instance->assigned ) {
+		argument_instance->assigned &&
+		!vips_pspec_value_is_null( pspec, value ) ) {
 		g_warning( "%s: %s can only assign '%s' once",
 			G_STRLOC,
 			G_OBJECT_TYPE_NAME( gobject ),
@@ -667,12 +712,6 @@ vips_object_set_property( GObject *gobject,
 		VIPS_SETSTR( *member, g_value_get_string( value ) );
 	}
 	else if( G_IS_PARAM_SPEC_OBJECT( pspec ) ) {
-		/* Remove any old object.
-		 */
-		vips_object_clear_object( object, pspec );
-
-		/* Install the new object.
-		 */
 		vips_object_set_object( object, pspec,
 			g_value_get_object( value ) );
 	}

libvips/iofuncs/operation.c

@@ -28,8 +28,8 @@
  */
 
 /*
-#define VIPS_DEBUG
  */
+#define VIPS_DEBUG
 
 #ifdef HAVE_CONFIG_H
 #include <config.h>