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

Yann Droneaud yann at droneaud.fr
Tue Oct 11 06:08:08 PDT 2011


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




More information about the xorg-devel mailing list