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

From: Heikki Linnakangas <hlinnakangas(at)vmware(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, PostgreSQL mailing lists <pgsql-odbc(at)postgresql(dot)org>
Subject: Re: Elimination of (more or less) all compilation warnings on OSX
Date: 2014-03-06 18:57:29
Message-ID: 5318C519.7020209@vmware.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-odbc

On 03/06/2014 06:50 PM, Tom Lane wrote:
> Heikki Linnakangas <hlinnakangas(at)vmware(dot)com> writes:
>> It's worth noting that isspace, isdigit etc. take an "unsigned char"
>> argument. The compiler won't warn if you pass a "char", so you have to
>> be careful with that. In practice, I don't think it will lead to actual
>> bugs on any real platform and locale, but still..
>
> It definitely used to lead to bugs on some old platforms. It may be
> that everybody is more careful nowadays, but the POSIX spec is still
> perfectly clear about it:
>
> The c argument is an int, the value of which the application
> shall ensure is a character representable as an unsigned char or
> equal to the value of the macro EOF. If the argument has any
> other value, the behavior is undefined.
>
> I had my old HPUX box hacked up so that gcc would produce "subscript
> is a char" warnings for unsafe usage. It seems a bit more difficult to
> get modern platforms to do that, though; people stopped exposing the
> underlying flag array in the system headers ...

A-ha, clever. I bet we could do something similar with modern compilers
too. Searching around, I found this trick (from
https://stackoverflow.com/questions/4712720/typechecking-macro-arguments-in-c)

> #define CHECK_TYPE(var, type) \
> { \
> __typeof(var) *__tmp; \
> __tmp = (type *) NULL; \
> (void) __tmp; /* to avoid "set but not used" warnings */ \
> }

The idea is to create a pointer variable that has the same type as the
argument, and assign an unsigned char value to it. With -Wpointer-sign,
gcc will complain. That's not portable, but that's OK. If we can get
some compilers to complain, that's enough.

With that, we can do:

#ifdef isalnum
#undef isalnum
#endif
#define isalnum(x) ((CHECK_TYPE(x, unsigned char)), isalnum(x))

I did that for all the is* routines, and unsurprisingly there were a
bunch of places were we got that wrong in psqlodbc. I'll fix those after
we get the minor release out.

I'll also try that on the PostgreSQL sources.

- Heikki

Attachment Content-Type Size
warn-on-signed-arg-to-isalnum-and-friends-1.patch text/x-diff 1.8 KB

In response to

Browse pgsql-odbc by date

  From Date Subject
Next Message Heikki Linnakangas 2014-03-06 19:10:55 Re: SQLFetchScroll with SQL_ATTR_ROWS_FETCHED_PTR closing statement.
Previous Message Przemyslaw Rzepecki 2014-03-06 17:05:07 Re: SQLFetchScroll with SQL_ATTR_ROWS_FETCHED_PTR closing statement.