API Shakeup: user data (was Re: [cairo] Patch improving fallbacks)

[Kristian Høgsberg]

Carl Worth wrote:
> On Tue, 15 Feb 2005 17:40:07 -0500, Kristian Høgsberg wrote:
> 
>>Argh, I guess I should have reviewed that one a bit more closely before 
>>sending it out.  Here a second try that doesn't remove the call to 
>>backend->destroy and has a full ChangeLog entry.
> 
> 
> OK. This looks pretty good to me. One change I'd like is to make the
> key argument be const friendly, (as suggested by Keith). The attached
> patch does this.

Yep, that makes sense.

> Then, the only thing I have is some nit-picky questions about how to
> name callback types and arguments:
> 
>>+typedef void (*cairo_destroy_func_t) (void *data);
> 
> I think I'd prefer to have the '*' in the argument list rather than
> swallowed up inside the typedef. I think this is consistent with my
> general distaste for pointer-swallowing typedefs. But there may be a
> compelling argument for a function-pointer exception to the
> rule. Anyone have one?

I really don't like pointer typedef's either, but all code that I've 
come across make that exception for function typedefs, so I didn't 
consider that option.  This works:

   typedef void cairo_destroy_func_t (void *data);

   cairo_status_t
   cairo_surface_set_user_data (cairo_surface_t *surface,
		     	       const cairo_user_data_key_t *key,
			       void *data,
			       cairo_destroy_func_t *destroy);

and gives the desired behaviour, but it looks somewhat unconventional to me.

> And "func" is an abbreviation I missed when I went hunting. Shall we
> just use "function" instead? I don't think the length is anything to
> worry about since the user never needs to type this name.

I did consider this, but decided to follow the style of the other 
function types in cairo.h.  Using 'function' instead sounds good to me.

> Finally, I wonder if it wouldn't help to get "user_data" into this
> type name to better suggest its association with the
> cairo_set_user_data call and the cairo_user_data_key_t type.

I also considered this, but my output stream patch also use a destroy 
function with the same type signature and for essentially the same 
purpose, so I was thinking that we could just call it 
cairo_destroy_function_t and share it between these two uses.

cheers,
Kristian