Обсуждение: 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.