Thread: Boatload of warnings in CVS HEAD :-(

I just noticed that my recent change to prevent PG_RE_THROW from dying
if there's noplace to longjmp to has provoked a whole lot of warnings
that were not there before.  Apparently this is because gcc understands
that siglongjmp() never returns, but is not equally clueful about
pg_re_throw().

We can fix this for gcc by putting __attribute__((noreturn)) on the
declaration of pg_re_throw(), but what about other compilers?
        regards, tom lane

Tom Lane wrote:
> 
> We can fix this for gcc by putting __attribute__((noreturn)) on the
> declaration of pg_re_throw(), but what about other compilers?
> 

Sun studio also complains about it :(.
    Zdenek

"Hannes Eder" <Hannes@HannesEder.net> writes:
> Tome Lane wrote:
>> We can fix this for gcc by putting __attribute__((noreturn)) on the
>> declaration of pg_re_throw(), but what about other compilers?

> For MSVC 2005 use __declspec(noreturn) (see [1]). I think this also work for some older versions of MSVC.

It might be too messy to try to do this for all compilers.  I thought of
a plan B, which is to make PG_RE_THROW() expand as
pg_re_throw(), exit(1)

which should be enough to persuade any compiler that understands the
concept at all.  Comments?
        regards, tom lane

Tome Lane wrote:
> We can fix this for gcc by putting __attribute__((noreturn)) on the
> declaration of pg_re_throw(), but what about other compilers?

For MSVC 2005 use __declspec(noreturn) (see [1]). I think this also work for some older versions of MSVC.

Regards,
Hannes Eder

References:
[1] http://msdn2.microsoft.com/en-us/library/k6ktzx3s(VS.80).aspx

Zdenek Kotala wrote:
> Tom Lane wrote:
>>
>> We can fix this for gcc by putting __attribute__((noreturn)) on the
>> declaration of pg_re_throw(), but what about other compilers?
>>
> 
> Sun studio also complains about it :(.
> 

I'm sorry it was to late for me, I recheck it again and Sun studio is
happy :-) and does not complaint about it, however there are a lot of
warning messages (not relevant with this issue). Most of them is about
following construct:

switch(..)
{
  case x :return(..);break;
...

Is the reason for keeping this in a code? Another kind of construct is:

#define PG_RETURN_NULL()  \    do { fcinfo->isnull = true; return (Datum) 0; } while (0)

It looks strange for me. Why it is used?

or

for(;;) { ... break;} see e.g
http://doxygen.postgresql.org/postgres_8c-source.html#l00198

or
why is there while ... break instead if?
http://doxygen.postgresql.org/comment_8c-source.html#l00221
    thanks for explanation        Zdenek

On Fri, May 04, 2007 at 02:18:31PM +0200, Zdenek Kotala wrote:
> Is the reason for keeping this in a code? Another kind of construct is:
>
> #define PG_RETURN_NULL()  \
>     do { fcinfo->isnull = true; return (Datum) 0; } while (0)

This is a standard way of getting multiple statements into a macro. If
the compiler complains, too bad, there isn't a standard alternative.

> for(;;) { ... break;} see e.g
> http://doxygen.postgresql.org/postgres_8c-source.html#l00198

So that within the loop you can use continue to start it again.

> or
> why is there while ... break instead if?
> http://doxygen.postgresql.org/comment_8c-source.html#l00221

Not sure about this one. It's not wrong, but it is unusual. Maybe
someone wanted to make it so that in the future it would handle
multiple cases?

Have a nice day,
--
Martijn van Oosterhout   <kleptog@svana.org>   http://svana.org/kleptog/
> From each according to his ability. To each according to his ability to litigate.

Martijn van Oosterhout wrote:
> On Fri, May 04, 2007 at 02:18:31PM +0200, Zdenek Kotala wrote:
> > Is the reason for keeping this in a code? Another kind of construct is:
> > 
> > #define PG_RETURN_NULL()  \
> >     do { fcinfo->isnull = true; return (Datum) 0; } while (0)
> 
> This is a standard way of getting multiple statements into a macro. If
> the compiler complains, too bad, there isn't a standard alternative.

So it is standard by it's not standard ;-)


> > or
> > why is there while ... break instead if?
> > http://doxygen.postgresql.org/comment_8c-source.html#l00221
> 
> Not sure about this one. It's not wrong, but it is unusual. Maybe
> someone wanted to make it so that in the future it would handle
> multiple cases?

There are many other cases where we do it cleanly, without the while
loop.  I don't see any reason to not do this one the same way.

- while ((oldtuple = systable_getnext(sd)) != NULL)
+ oldtuple = systable_getnext(sd);
+ if (HeapTupleIsValid(oldtuple))
...
- break;

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.