Re: Elimination of (more or less) all compilation warnings on OSX

On 03/06/2014 03:10 PM, Michael Paquier wrote:
> Hi all,
>
> Before beginning my real quest for odbc, please find attached a patch
> that cleans up 99% of the code warnings I saw on OSX (and actually
> some of them on Windows). All those warnings are caused by incorrect
> casts, so fixing them is trivial... But there were quite a lot of
> them, I think I fixed 100~200 of them...

Those warnings are the reason I added -Wno-pointer-sign to the compiler
options. Looks like your compiler on OS X doesn't recognize that.

In addition to being ugly, suppressing the warnings with casts can make
you miss genuine bugs if you change the type of a variable. For example,
if you have something like this:

char *foo;
unsigned char *bar;
...
foo = bar;

And you the definition of 'bar' to:

struct mystruct *bar;

With -Wno-pointer-sign, the compiler will still warn you about that. But
if you had suppressed that by:

foo = (char *) bar;

you will not get a warning.

So I'm generally not in favor of just sprinkling new casts to suppress
these warnings. In some cases a cast is the best solution, for sure, but
in general we should rather be consistent with the pointer types we use.
For example, attached is a patch that fixes up convert.c. It actually
*removes* one such cast, which is no longer necessary. That's more work
than just adding casts, but that's what we should do, if anything.

We're bound to need some casts, because the ODBC API tends to use
unsigned char pointers while all the libc string functions use signed.
But that should be limited to the interface stuff. For example,
SQLExecDirect has to use an unsigned "SQLCHAR *" in the function
signature, and PGAPI_ExecDirect is the implementation of that, but in
PGAPI_ExecDirect, after we convert the SQLCHAR * and length to a string,
we can deal with it as a signed pointer.

Actually, just changing the signature of make_string slashes the number
of warnings significantly:

-char       *make_string(const char *s, ssize_t len, char *buf, size_t
bufsize);
+char       *make_string(const SQLCHAR *s, SQLINTEGER len, char *buf,
size_t bufsize);

- Heikki