Ok, thanks for addressing my concerns.
Reviewed-by: Jeremy Huddleston <jerem...@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@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
_______________________________________________
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
