[Xcb] [PATCH xcb] Added more error states and removed global error_connection

Arvind Umrao arvind.umrao at oracle.com
Fri Oct 28 20:42:27 PDT 2011


On 10/28/11 23:01, Uli Schlachter wrote:
> On 28.10.2011 17:16, Arvind Umrao wrote:
> [...]
>> +xcb_connection_t *_xcb_conn_ret_error(int err)
>>   {
>> -    c->has_error = 1;
>> +    static int error_connection;
>> +    error_connection = err;
>> +    return (xcb_connection_t *)&error_connection ;
>>   }
> [...]
>
> That doesn't look thread-safe to me.
Yes I agree with you, it is not thread-safe and I thought in that 
dimension. But from user's point of perspective, type of error does not 
matter much,  whatever final cause of connection error from any thread, 
user need to fix it first and retry. In most cases, error state will be 
same in all the threads.  After deep rethinking if you still do not 
agree with my point of view, I will give you one more patch with 
changes, you suggested.

>   If you really want to go that way, you need
> a seperate static variable for every possible error.
Yes I agree with you,  but original code also need the same changes. 
Right now, error_connection is hardcoded to 1, but there are 
possibilities of other error states. We have to set the other error 
states in global file, which will look even more ugly.

For eg.
constant int error_connection_mem_unsufficent =2;
constant int error_connection_FD_over_flow=3;

I will go with your idea and define a seperate static variable for every 
possible error, inside one function, which will look much better than 
doing it at public header file.

> For https://bugs.freedesktop.org/show_bug.cgi?id=42304:
> Do you know what causes that bug? Why does gcc complain about a static global
> variable, but a static local one works fine?

I am getting following error without my code changes when I compile code 
with gcc 4.3.3 on Solaris.   Even if we forget about compilation error,  
then also we have to remove  "error_connection =1". There are 
possibilities of other error states, as I have mentioned above.

gcc -shared -Wl,-h -Wl,libxcb.so.1 -o .libs/libxcb.so.1.1.0  
.libs/xcb_conn.o .libs/xcb_out.o .libs/xcb_in.o .libs/xcb_ext.o 
.libs/xcb_xid.o .libs/xcb_list.o .libs/xcb_util.o .libs/xcb_auth.o 
.libs/xproto.o .libs/bigreq.o .libs/xc_misc.o  
-R/export/work/git/xorg/build/lib -R/export/work/git/xorg/build/lib 
-L/export/work/git/xorg/build/lib 
/export/work/git/xorg/build/lib/libXau.so 
/export/work/git/xorg/build/lib/libXdmcp.so -lsocket -lc
ld: fatal: relocation error: R_386_GOTOFF: file .libs/xcb_conn.o: symbol 
error_connection: a GOT relative relocation must reference a local symbol
ld: fatal: relocation error: R_386_GOTOFF: file .libs/xcb_conn.o: symbol 
error_connection: a GOT relative relocation must reference a local symbol
ld: fatal: relocation error: R_386_GOTOFF: file .libs/xcb_conn.o: symbol 
error_connection: a GOT relative relocation must reference a local symbol
collect2: ld returned 1 exit status
*** Error code 1
make: Fatal error: Command failed for target `libxcb.la'
Current working directory /export/work/git/xorg/src/xcb/libxcb/src

> Uli
>



More information about the Xcb mailing list