Fixing compiler warnings

[Adam Jackson]

On Mon, 2009-02-02 at 23:02 +0100, Tomas Carnecky wrote:

> ----------------------------------------------------------------------
> bigreq.c:46: warning: no previous prototype for ‘BigReqExtensionInit’
> sync.c:2124: warning: no previous prototype for ‘SyncExtensionInit’
> xcmisc.c:59: warning: no previous prototype for ‘XCMiscExtensionInit’
> 
> These three extensions are the only ones that have no previous 
> prototype. It seems as the other extensions have their prototype from 
> hw/xfree86/dixmods/extmod/modinit.h. It seems a bit strange that Xext/ 
> modules include a file from hw/. But I'm not that familiar with the 
> dependencies so I'll leave it up to you to decide what's to be done here.

Yeah, those (and all like them) should come from include/ top level
instead of hw/.  There doesn't seem to be a sensible place already, so I
nominate include/extinit.h as being obviously named.

The other two function protos in extinit.h should move in the process.
input.h probably?

> ----------------------------------------------------------------------
> glapitemp.h:1884: warning: ‘NoOp_dispatch_stub_339’ defined but not used
> (repeated many times for different stubs)
> 
> The file seems to have a mechanism to silence the compiler:
> /*
>   * This is just used to silence compiler warnings.
>   * We list the functions which are not otherwise used.
>   */
> 
> Maybe the gl_apitemp.py scripts needs to be updated and the file 
> regenerated?

Probably.  I wouldn't worry too much about those warnings, the compiler
will optimize the functions out for you, but would be nice to clean up.

> ----------------------------------------------------------------------
> warning: cast to pointer from integer of different size
> warning: cast from function call of type ‘pointer’ to non-matching type 
> ‘long unsigned int’
> 
> This is because the code casts integers to pointers and back. Most of 
> the time the code just wants to save an int in the private data, and 
> uses something along these lines:
> 
>    int i;
>    dixSetPrivate(&devPrivates, myKey, (pointer) i);
>    i = dixGetPrivate(&devPrivates, myKey);
> 
> Should the code use (u)intptr_t in these cases?

We're not broadly assuming the existence of stdint.h yet.  But we
certainly should.  If you felt like rewriting Xmd.h in terms of same...

And then, yeah, uintptr_t all the way.

> In three cases the warning is about code in hw/xfree86/dri/dri.c casting 
> a void pointer to drm_handle_t which is 'unsigned int'.

This is an unfortunate bit of ABI.  Not sure how to shut up the warnings
in this case.

> ----------------------------------------------------------------------
> sdksyms.c:1056: warning: initialization discards qualifiers from pointer 
> target type
> 
> It seems because isItTimeToYield is declared volatile? Is it OK to 
> explicitly cast that symbol to 'void *'?

In the symbol export list, yes.

> ----------------------------------------------------------------------
> Finally some warnings about deprecated functions. I don't think there's 
> anything that can be done with it except eventually removing the 
> functions, right?

Right.

> ----------------------------------------------------------------------
> One warning in xf86Bus.c:x_isSubsetOf() which I reported in an earlier 
> mail. Maybe nobody cares about the code anymore. It hasn't been touched 
> since the initial import.

xf86IsSubsetOf() isn't used.  Anywhere.  And that's the only caller.
Kill 'em with fire.

- ajax
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part
URL: <http://lists.x.org/archives/xorg/attachments/20090204/a9e2e151/attachment.pgp>