[PATCH/libX11] Return name instead of value in XGetIMValues() and XSetIMValues()

[Jeremy Huddleston]

Ok, thanks for addressing my concerns.

Reviewed-by: Jeremy Huddleston <jeremyhu at apple.com>

On Oct 11, 2011, at 06:08, Yann Droneaud wrote:

> Le lundi 10 octobre 2011 à 14:00 -0700, Jeremy Huddleston a écrit :
>> The src changes look right, but I'm wondering if this is a documentation bug rather than an implementation bug. 
> 
> Returning the value is not really useful to diagnose an error,
> a program would have to manage a conversion table from value to
> parameter in order to produce an useful error message.
> 
>> Is anyone relying on the current (undocumented) behavior?
>> 
> 
> AFAIK, after searching code through Google Codesearch,
> XGetIMValues() is usually used like this:
> 
>   XIMStyles *styles = NULL;
> 
>   if (XGetIMValues(xim, XNQueryInputStyle, &styles, NULL) != NULL
>       || styles == NULL) {
> 	/* error case */
>   } else {
> 	XFree(styles);
>   }
> 
> I have only found one usage concerned by the proposed change
> 
> http://www.google.com/codesearch#av_p04Hv9fs/src/solaris/native/sun/awt/awt_InputMethod.c&q=XGetIMValues%20case:yes&sq=&ct=rc&cd=12
> 
> 
>   ret = XGetIMValues(X11im, XNQueryInputStyle, &im_styles, NULL);
> 
>   if (ret != NULL) {
>       jio_fprintf(stderr,"XGetIMValues: %s\n",ret);
>       return FALSE ;
>   }
> 
> 
> Without my patch, I think this code is clearly not useful.
> 
> Regarding XSetIMValues(), it usually used like this
> 
> 
>   XIMCallback callback;
> 
>   callback.client_data = NULL;
>   callback.callback = im_destroy;
> 
>   if (XSetIMValues(xim, XNDestroyCallback, &imcallback, NULL) != NULL) {
>      /* error */
>   }
> 
> 
> Using Google Codesearch, I wasn't able to found any other uses of XSetIMValues():
> it seems no one it using the return value apart testing it against NULL.
> 
> Regards.
> 
> -- 
> Yann Droneaud
> 
> 
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel