Re: [HACKERS] Compiler warning in costsize.c
On 11 April 2017 at 12:53, Michael Paquier wrote: > On Tue, Apr 11, 2017 at 4:02 AM, Robert Haas wrote: >> On Mon, Apr 10, 2017 at 2:09 PM, Tom Lane wrote: >>> ilm...@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) writes: Why bother with the 'rte' variable at all if it's only used for the Assert()ing the rtekind? >>> >>> That was proposed a few messages back. I don't like it because it makes >>> these functions look different from the other scan-cost-estimation >>> functions, and we'd just have to undo the "optimization" if they ever >>> grow a need to reference the rte for another purpose. >> >> I think that's sort of silly, though. It's a trivial difference, >> neither likely to confuse anyone nor difficult to undo. > > +1. I would just do that and call it a day. There is no point to do a > mandatory list lookup as that's just for an assertion, and fixing this > warning does not seem worth the addition of fancier facilities. If the > function declarations were doubly-nested in the code, I would > personally consider the use of a variable, but not here. Any more thoughts on what is acceptable for fixing this? beta1 is looming and it seems a bit messy to be shipping that with these warnings, however harmless they are. -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] compiler warning with VS 2017
Petr Jelinek writes: > On 05/05/17 16:56, Tom Lane wrote: >> So the comment should be something like "if the column is unchanged, >> we should not attempt to access its value beyond this point. To >> help catch any such attempts, set the string to NULL" ? > Yes that sounds about right. We don't get any data for unchanged TOAST > columns (that's limitation of logical decoding) so we better not touch them. OK. I just made it say "we don't get the value of unchanged columns". regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] compiler warning with VS 2017
On 05/05/17 16:56, Tom Lane wrote: > Petr Jelinek writes: >> On 05/05/17 06:50, Tom Lane wrote: >>> Actually, looking around a bit there, it's not even clear why >>> we should be booby-trapping the value of an unchanged column in >>> the first place. So I'd say that not only is the code dubious >>> but the comment is inadequate too. > >> Hmm, as far as I can recollect this is just leftover debugging code that >> was intended to help ensure that we are checking the "changed" >> everywhere we are supposed to (since I changed handling of these >> structured quite a bit during development). Should be changed to NULL, >> that's what we usually do in this type of situation. > > So the comment should be something like "if the column is unchanged, > we should not attempt to access its value beyond this point. To > help catch any such attempts, set the string to NULL" ? > Yes that sounds about right. We don't get any data for unchanged TOAST columns (that's limitation of logical decoding) so we better not touch them. -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] compiler warning with VS 2017
Petr Jelinek writes: > On 05/05/17 06:50, Tom Lane wrote: >> Actually, looking around a bit there, it's not even clear why >> we should be booby-trapping the value of an unchanged column in >> the first place. So I'd say that not only is the code dubious >> but the comment is inadequate too. > Hmm, as far as I can recollect this is just leftover debugging code that > was intended to help ensure that we are checking the "changed" > everywhere we are supposed to (since I changed handling of these > structured quite a bit during development). Should be changed to NULL, > that's what we usually do in this type of situation. So the comment should be something like "if the column is unchanged, we should not attempt to access its value beyond this point. To help catch any such attempts, set the string to NULL" ? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] compiler warning with VS 2017
On 05/05/17 06:50, Tom Lane wrote: > Haribabu Kommi writes: >> I am getting a compiler warning when I build the latest HEAD PostgreSQL with >> visual studio 2017. >> The code at the line is, >> tuple->values[i] = (char *) (Size)0xdeadbeef; /* make bad usage more obvious >> */ > > Yeah, you're not the first to complain about this. To my mind that > coding is not pretty, not cute, and not portable: there's not even > a good reason to believe that dereferencing the pointer would result > in a crash. Perhaps the author can explain to us why this is better > than just assigning NULL. > > Actually, looking around a bit there, it's not even clear why > we should be booby-trapping the value of an unchanged column in > the first place. So I'd say that not only is the code dubious > but the comment is inadequate too. Hmm, as far as I can recollect this is just leftover debugging code that was intended to help ensure that we are checking the "changed" everywhere we are supposed to (since I changed handling of these structured quite a bit during development). Should be changed to NULL, that's what we usually do in this type of situation. -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] compiler warning with VS 2017
Haribabu Kommi writes: > I am getting a compiler warning when I build the latest HEAD PostgreSQL with > visual studio 2017. > The code at the line is, > tuple->values[i] = (char *) (Size)0xdeadbeef; /* make bad usage more obvious > */ Yeah, you're not the first to complain about this. To my mind that coding is not pretty, not cute, and not portable: there's not even a good reason to believe that dereferencing the pointer would result in a crash. Perhaps the author can explain to us why this is better than just assigning NULL. Actually, looking around a bit there, it's not even clear why we should be booby-trapping the value of an unchanged column in the first place. So I'd say that not only is the code dubious but the comment is inadequate too. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] compiler warning with VS 2017
I am getting a compiler warning when I build the latest HEAD PostgreSQL with visual studio 2017. src/backend/replication/logical/proto.c(482): warning C4312: 'type cast': conversion from 'unsigned int' to 'char *' of greater size Details of the warning is available in the link [1]. The code at the line is, tuple->values[i] = (char *) (Size)0xdeadbeef; /* make bad usage more obvious */ If I change the code as (char *) (Size)0xdeadbeef; or (char *) (INT_PTR)0xdeadbeef; the warning disappears. How about fixing it as below and add the typecast or disable this warning? /* * PTR_CAST * Used when converting integer addresses to a pointer. * The casting is used to avoid generating warning in * windows */ #ifdef WIN32 #define PTR_CAST INT_PTR #else #define PTR_CAST #endif [1] - https://msdn.microsoft.com/en-us/library/h97f4b9y.aspx Regards, Hari Babu Fujitsu Australia
Re: [HACKERS] Compiler warning in costsize.c
On Tue, Apr 11, 2017 at 4:02 AM, Robert Haas wrote: > On Mon, Apr 10, 2017 at 2:09 PM, Tom Lane wrote: >> ilm...@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) writes: >>> Why bother with the 'rte' variable at all if it's only used for the >>> Assert()ing the rtekind? >> >> That was proposed a few messages back. I don't like it because it makes >> these functions look different from the other scan-cost-estimation >> functions, and we'd just have to undo the "optimization" if they ever >> grow a need to reference the rte for another purpose. > > I think that's sort of silly, though. It's a trivial difference, > neither likely to confuse anyone nor difficult to undo. +1. I would just do that and call it a day. There is no point to do a mandatory list lookup as that's just for an assertion, and fixing this warning does not seem worth the addition of fancier facilities. If the function declarations were doubly-nested in the code, I would personally consider the use of a variable, but not here. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Compiler warning in costsize.c
On Mon, Apr 10, 2017 at 2:09 PM, Tom Lane wrote: > ilm...@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) writes: >> Why bother with the 'rte' variable at all if it's only used for the >> Assert()ing the rtekind? > > That was proposed a few messages back. I don't like it because it makes > these functions look different from the other scan-cost-estimation > functions, and we'd just have to undo the "optimization" if they ever > grow a need to reference the rte for another purpose. I think that's sort of silly, though. It's a trivial difference, neither likely to confuse anyone nor difficult to undo. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Compiler warning in costsize.c
ilm...@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) writes: > Why bother with the 'rte' variable at all if it's only used for the > Assert()ing the rtekind? That was proposed a few messages back. I don't like it because it makes these functions look different from the other scan-cost-estimation functions, and we'd just have to undo the "optimization" if they ever grow a need to reference the rte for another purpose. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Compiler warning in costsize.c
Robert Haas writes: > On Mon, Apr 10, 2017 at 8:30 AM, Michael Paquier > wrote: >> On Mon, Apr 10, 2017 at 9:05 PM, Tom Lane wrote: >>> I wonder if we shouldn't just do >>> ... >>> and eat the "useless" calculation of rte. > -1 from me. I'm not a big fan of useless calculation just because it > happens to be needed in an Assert-enabled build. Well, those planner_rt_fetch() calls are going to reduce to a simple array lookup, so it seems rather extreme to insist on contorting the code just to avoid that. It's not like these functions are trivially cheap otherwise. In fact, I kind of wonder why we're using planner_rt_fetch() at all in costsize.c, rather than "root->simple_rte_array[rel->relid]". Maybe at one time these functions were invokable before reaching query_planner(), but we don't do that anymore. (Just to be sure, I stuck "Assert(root->simple_rte_array)" into each costsize.c function that uses planner_rt_fetch, and it still passes check-world.) So now my proposal is /* Should only be applied to base relations that are subqueries */ Assert(rel->relid > 0); -#ifdef USE_ASSERT_CHECKING - rte = planner_rt_fetch(rel->relid, root); + rte = root->simple_rte_array[rel->relid]; Assert(rte->rtekind == RTE_SUBQUERY); -#endif and make the rest of costsize.c look like that too. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Compiler warning in costsize.c
Tom Lane writes: > I wonder if we shouldn't just do > > RangeTblEntry *rte PG_USED_FOR_ASSERTS_ONLY; > ListCell *lc; > > /* Should only be applied to base relations that are subqueries */ > Assert(rel->relid > 0); > -#ifdef USE_ASSERT_CHECKING > rte = planner_rt_fetch(rel->relid, root); > Assert(rte->rtekind == RTE_SUBQUERY); > -#endif > > and eat the "useless" calculation of rte. Why bother with the 'rte' variable at all if it's only used for the Assert()ing the rtekind? Assert(planner_rt_fetch(rel->relid, root)->rtekind == RTE_SUBQUERY); - ilmari -- "The surreality of the universe tends towards a maximum" -- Skud's Law "Never formulate a law or axiom that you're not prepared to live with the consequences of." -- Skud's Meta-Law -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Compiler warning in costsize.c
On Mon, Apr 10, 2017 at 8:30 AM, Michael Paquier wrote: > On Mon, Apr 10, 2017 at 9:05 PM, Tom Lane wrote: >> I wonder if we shouldn't just do >> >> RangeTblEntry *rte PG_USED_FOR_ASSERTS_ONLY; >> ListCell *lc; >> >> /* Should only be applied to base relations that are subqueries */ >> Assert(rel->relid > 0); >> -#ifdef USE_ASSERT_CHECKING >> rte = planner_rt_fetch(rel->relid, root); >> Assert(rte->rtekind == RTE_SUBQUERY); >> -#endif >> >> and eat the "useless" calculation of rte. > > That works as well. Now this code really has been written so as we > don't want to do this useless computation for non-Assert builds, > that's why I did not suggest it. But as it does just a list_nth call, > that's not really costly... And other code paths dealing with the cost > do it as well. -1 from me. I'm not a big fan of useless calculation just because it happens to be needed in an Assert-enabled build. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Compiler warning in costsize.c
On Mon, Apr 10, 2017 at 9:05 PM, Tom Lane wrote: > I wonder if we shouldn't just do > > RangeTblEntry *rte PG_USED_FOR_ASSERTS_ONLY; > ListCell *lc; > > /* Should only be applied to base relations that are subqueries */ > Assert(rel->relid > 0); > -#ifdef USE_ASSERT_CHECKING > rte = planner_rt_fetch(rel->relid, root); > Assert(rte->rtekind == RTE_SUBQUERY); > -#endif > > and eat the "useless" calculation of rte. That works as well. Now this code really has been written so as we don't want to do this useless computation for non-Assert builds, that's why I did not suggest it. But as it does just a list_nth call, that's not really costly... And other code paths dealing with the cost do it as well. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Compiler warning in costsize.c
David Rowley writes: > On 8 April 2017 at 04:42, Tom Lane wrote: >> BTW, is it really true that only these two places produce such warnings >> on MSVC? I see about three dozen uses of PG_USED_FOR_ASSERTS_ONLY in our >> tree, and I'd have expected all of those places to be causing warnings on >> a compiler that doesn't have a way to understand that annotation. > Seems that MSVC is happy once the variable is assigned, and does not > bother checking if the variable is used after being assigned, whereas, > some other compilers might see the variable as uselessly assigned. > At the moment there are no other warnings from MSVC since all the > other places the variable gets assigned a value in some code path. I wonder if we shouldn't just do RangeTblEntry *rte PG_USED_FOR_ASSERTS_ONLY; ListCell *lc; /* Should only be applied to base relations that are subqueries */ Assert(rel->relid > 0); -#ifdef USE_ASSERT_CHECKING rte = planner_rt_fetch(rel->relid, root); Assert(rte->rtekind == RTE_SUBQUERY); -#endif and eat the "useless" calculation of rte. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Compiler warning in costsize.c
On 8 April 2017 at 04:42, Tom Lane wrote: > I'd be happier with something along the line of > > RangeTblEntry *rte; > ListCell *lc; > > /* Should only be applied to base relations that are subqueries */ > Assert(rel->relid > 0); > rte = planner_rt_fetch(rel->relid, root); > #ifdef USE_ASSERT_CHECKING > Assert(rte->rtekind == RTE_SUBQUERY); > #else > (void) rte; /* silence unreferenced-variable complaints */ > #endif the (void) rte; would not be required to silence MSVC here. Of course, PG_USED_FOR_ASSERTS_ONLY would be required to stop some smarter compiler from complaining. > assuming that that actually does silence the warning on MSVC. > > BTW, is it really true that only these two places produce such warnings > on MSVC? I see about three dozen uses of PG_USED_FOR_ASSERTS_ONLY in our > tree, and I'd have expected all of those places to be causing warnings on > a compiler that doesn't have a way to understand that annotation. Seems that MSVC is happy once the variable is assigned, and does not bother checking if the variable is used after being assigned, whereas, some other compilers might see the variable as uselessly assigned. At the moment there are no other warnings from MSVC since all the other places the variable gets assigned a value in some code path. -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Compiler warning in costsize.c
Michael Paquier writes: > Bah. This actually fixes nothing. Attached is a different patch that > really addresses the problem, by removing the variable because we > don't want planner_rt_fetch() to run for non-Assert builds. I don't really like any of these fixes, because they take the code further away from the structure used by all the other similar functions in costsize.c, and they will be hard to undo whenever these functions grow a reason to look at the RTE normally (outside asserts). I'd be happier with something along the line of RangeTblEntry *rte; ListCell *lc; /* Should only be applied to base relations that are subqueries */ Assert(rel->relid > 0); rte = planner_rt_fetch(rel->relid, root); #ifdef USE_ASSERT_CHECKING Assert(rte->rtekind == RTE_SUBQUERY); #else (void) rte; /* silence unreferenced-variable complaints */ #endif assuming that that actually does silence the warning on MSVC. BTW, is it really true that only these two places produce such warnings on MSVC? I see about three dozen uses of PG_USED_FOR_ASSERTS_ONLY in our tree, and I'd have expected all of those places to be causing warnings on a compiler that doesn't have a way to understand that annotation. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Compiler warning in costsize.c
On Fri, Apr 7, 2017 at 12:38 AM, Michael Paquier wrote: > On Tue, Apr 4, 2017 at 9:42 PM, Michael Paquier > wrote: >> On Wed, Apr 5, 2017 at 2:54 AM, Tom Lane wrote: >>> (I'm personally not that much in love with PG_USED_FOR_ASSERTS_ONLY, >>> because it tends to confuse pgindent.) >> >> I would be incline to just do that, any other solution I can think of >> is uglier than that. > > Actually, no. Looking at this issue again the warning is triggered > because the Assert() clause is present in USE_ASSERT_CHECKING. So > instead of removing PG_USED_FOR_ASSERTS_ONLY, here is a more simple > patch that fixes the problem. No need to put the variable *rte within > ifdefs as well. Bah. This actually fixes nothing. Attached is a different patch that really addresses the problem, by removing the variable because we don't want planner_rt_fetch() to run for non-Assert builds. -- Michael costsize-warning-fix-3.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Compiler warning in costsize.c
On Tue, Apr 4, 2017 at 9:42 PM, Michael Paquier wrote: > On Wed, Apr 5, 2017 at 2:54 AM, Tom Lane wrote: >> (I'm personally not that much in love with PG_USED_FOR_ASSERTS_ONLY, >> because it tends to confuse pgindent.) > > I would be incline to just do that, any other solution I can think of > is uglier than that. Actually, no. Looking at this issue again the warning is triggered because the Assert() clause is present in USE_ASSERT_CHECKING. So instead of removing PG_USED_FOR_ASSERTS_ONLY, here is a more simple patch that fixes the problem. No need to put the variable *rte within ifdefs as well. -- Michael costsize-warning-fix-2.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Compiler warning in costsize.c
On Wed, Apr 5, 2017 at 2:54 AM, Tom Lane wrote: > Michael Paquier writes: >> In builds where USE_ASSERT_CHECKING is not enabled, costsize.c can >> generate warnings. Here is for example with MSVC: >> src/backend/optimizer/path/costsize.c(4520): warning C4101: 'rte' : >> unreferenced local variable [C:\Users\ioltas\git\postgres\postgres.vcxproj] >> src/backend/optimizer/path/costsize.c(4640): warning C4101: 'rte' : >> unreferenced local variable [C:\Users\ioltas\git\postgres\postgres.vcxproj] > >> a9c074ba7 has done an effort, but a bit more is needed as the attached. > > That doesn't look right at all: > > +#ifdef USE_ASSERT_CHECKING > RangeTblEntry *rte PG_USED_FOR_ASSERTS_ONLY; > +#endif > > The entire point of the PG_USED_FOR_ASSERTS_ONLY annotation is to suppress > this type of warning without a need for an explicit #ifdef like that one. > > We should either fix PG_USED_FOR_ASSERTS_ONLY to work on MSVC as well, > or decide that we don't care about suppressing such warnings on MSVC, > or give up on PG_USED_FOR_ASSERTS_ONLY and remove it everywhere in > favor of plain #ifdefs. Visual does not have any equivalent of __attribute__((unused))... And visual does not have an equivalent after looking around. A trick that I can think of would be to change PG_USED_FOR_ASSERTS_ONLY to be roughly a macro like that: #if defined(__GNUC__) #define PG_ASSERT_ONLY(x) ASSERT_ONLY_ ## x __attribute__((unused)) #else #define PG_ASSERT_ONLY(x) ((void) x) #endif But that's ugly... > (I'm personally not that much in love with PG_USED_FOR_ASSERTS_ONLY, > because it tends to confuse pgindent.) I would be incline to just do that, any other solution I can think of is uglier than that. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Compiler warning in costsize.c
Michael Paquier writes: > In builds where USE_ASSERT_CHECKING is not enabled, costsize.c can > generate warnings. Here is for example with MSVC: > src/backend/optimizer/path/costsize.c(4520): warning C4101: 'rte' : > unreferenced local variable [C:\Users\ioltas\git\postgres\postgres.vcxproj] > src/backend/optimizer/path/costsize.c(4640): warning C4101: 'rte' : > unreferenced local variable [C:\Users\ioltas\git\postgres\postgres.vcxproj] > a9c074ba7 has done an effort, but a bit more is needed as the attached. That doesn't look right at all: +#ifdef USE_ASSERT_CHECKING RangeTblEntry *rte PG_USED_FOR_ASSERTS_ONLY; +#endif The entire point of the PG_USED_FOR_ASSERTS_ONLY annotation is to suppress this type of warning without a need for an explicit #ifdef like that one. We should either fix PG_USED_FOR_ASSERTS_ONLY to work on MSVC as well, or decide that we don't care about suppressing such warnings on MSVC, or give up on PG_USED_FOR_ASSERTS_ONLY and remove it everywhere in favor of plain #ifdefs. (I'm personally not that much in love with PG_USED_FOR_ASSERTS_ONLY, because it tends to confuse pgindent.) regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Compiler warning in costsize.c
On Tue, Apr 4, 2017 at 7:03 PM, David Rowley wrote: > On 4 April 2017 at 16:22, Michael Paquier wrote: >> Hi all, >> >> In builds where USE_ASSERT_CHECKING is not enabled, costsize.c can >> generate warnings. Here is for example with MSVC: >> src/backend/optimizer/path/costsize.c(4520): warning C4101: 'rte' : >> unreferen >> ced local variable [C:\Users\ioltas\git\postgres\postgres.vcxproj] >> src/backend/optimizer/path/costsize.c(4640): warning C4101: 'rte' : >> unreferen >> ced local variable [C:\Users\ioltas\git\postgres\postgres.vcxproj] >> >> a9c074ba7 has done an effort, but a bit more is needed as the attached. > > Thanks for writing the patch. > > I wondering if it would be worth simplifying it a little to get rid of > the double #ifdefs, as per attached. Yes, that would be fine as well. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Compiler warning in costsize.c
On 4 April 2017 at 16:22, Michael Paquier wrote: > Hi all, > > In builds where USE_ASSERT_CHECKING is not enabled, costsize.c can > generate warnings. Here is for example with MSVC: > src/backend/optimizer/path/costsize.c(4520): warning C4101: 'rte' : > unreferen > ced local variable [C:\Users\ioltas\git\postgres\postgres.vcxproj] > src/backend/optimizer/path/costsize.c(4640): warning C4101: 'rte' : > unreferen > ced local variable [C:\Users\ioltas\git\postgres\postgres.vcxproj] > > a9c074ba7 has done an effort, but a bit more is needed as the attached. Thanks for writing the patch. I wondering if it would be worth simplifying it a little to get rid of the double #ifdefs, as per attached. -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services costsize-fix-warning_drowley.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Compiler warning in costsize.c
Hi all, In builds where USE_ASSERT_CHECKING is not enabled, costsize.c can generate warnings. Here is for example with MSVC: src/backend/optimizer/path/costsize.c(4520): warning C4101: 'rte' : unreferen ced local variable [C:\Users\ioltas\git\postgres\postgres.vcxproj] src/backend/optimizer/path/costsize.c(4640): warning C4101: 'rte' : unreferen ced local variable [C:\Users\ioltas\git\postgres\postgres.vcxproj] a9c074ba7 has done an effort, but a bit more is needed as the attached. Thanks, -- Michael costsize-fix-warning.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] compiler warning in set_tablefunc_size_estimates
On Thu, Mar 9, 2017 at 4:39 PM, Tom Lane wrote: > Robert Haas writes: >> I tried a non-cassert compile on a machine that has a pickier compiler >> than my laptop, and got: > >> costsize.c: In function ‘set_tablefunc_size_estimates’: >> costsize.c:4574:17: error: variable ‘rte’ set but not used >> [-Werror=unused-but-set-variable] > >> That appears to be a legitimate gripe. Perhaps: > > I think PG_USED_FOR_ASSERTS_ONLY would be a better solution. It's > only happenstance that the function currently uses the RTE just > for this; if it grows another use, your approach would be harder > to clean up. Yeah, we might have to revert the entire -4/+1 line patch. Actually, the thing I don't like about that is that that then we're still emitting code for the planner_rt_fetch. That probably doesn't cost much, but why do it? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] compiler warning in set_tablefunc_size_estimates
Robert Haas writes: > I tried a non-cassert compile on a machine that has a pickier compiler > than my laptop, and got: > costsize.c: In function ‘set_tablefunc_size_estimates’: > costsize.c:4574:17: error: variable ‘rte’ set but not used > [-Werror=unused-but-set-variable] > That appears to be a legitimate gripe. Perhaps: I think PG_USED_FOR_ASSERTS_ONLY would be a better solution. It's only happenstance that the function currently uses the RTE just for this; if it grows another use, your approach would be harder to clean up. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] compiler warning in set_tablefunc_size_estimates
I tried a non-cassert compile on a machine that has a pickier compiler than my laptop, and got: costsize.c: In function ‘set_tablefunc_size_estimates’: costsize.c:4574:17: error: variable ‘rte’ set but not used [-Werror=unused-but-set-variable] That appears to be a legitimate gripe. Perhaps: diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c index e78f3a8..c23f681 100644 --- a/src/backend/optimizer/path/costsize.c +++ b/src/backend/optimizer/path/costsize.c @@ -4571,12 +4571,9 @@ set_function_size_estimates(PlannerInfo *root, RelOptInfo *rel) void set_tablefunc_size_estimates(PlannerInfo *root, RelOptInfo *rel) { - RangeTblEntry *rte; - /* Should only be applied to base relations that are functions */ Assert(rel->relid > 0); - rte = planner_rt_fetch(rel->relid, root); - Assert(rte->rtekind == RTE_TABLEFUNC); + Assert(planner_rt_fetch(rel->relid, root)->rtekind == RTE_TABLEFUNC); rel->tuples = 100; -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Compiler warning
On Dec 24, 2016 01:21, "Bruce Momjian" wrote: I am seeing this compiler warning in the 9.4 branch: 9.4: basebackup.c:1284:6: warning: variable 'wait_result' set but not used [-Wunused-but-set-variable] This is on Debian Jessie with gcc version 4.9.2. It is from this commit: commit f6508827afe76b2c3735a9ce073620e708d60c79 Hi! This was already reported by Dean back on the thread on - committers, including one question still to be investigated. I plan to get back to it when I get back from Christmas holidays. /Magnus
[HACKERS] Compiler warning
I am seeing this compiler warning in the 9.4 branch: 9.4: basebackup.c:1284:6: warning: variable 'wait_result' set but not used [-Wunused-but-set-variable] This is on Debian Jessie with gcc version 4.9.2. It is from this commit: commit f6508827afe76b2c3735a9ce073620e708d60c79 Author: Magnus Hagander Date: Mon Dec 19 10:11:04 2016 +0100 Fix base backup rate limiting in presence of slow i/o When source i/o on disk was too slow compared to the rate limiting specified, the system could end up with a negative value for sleep that it never got out of, which caused rate limiting to effectively be turned off. Discussion: https://postgr.es/m/CABUevEy_-e0YvL4ayoX8bH_Ja9w%2BBHoP6jUgdxZuG2nEj3uAfQ%40mail.gmail.com Analysis by me, patch by Antonin Houska -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] compiler warning read_objtype_from_string()
Tom Lane wrote: > I do not think you should assume that the compiler is smart enough to > deduce that, nor that all compilers even know ereport(ERROR) doesn't > return. Personally I don't see the point of the "type" variable at > all, anyway. I would have written this as > > [code] Makes sense. I will patch it that way. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] compiler warning read_objtype_from_string()
Alvaro Herrera writes: > Peter Eisentraut wrote: >> I'm getting the following compiler warning (using nondefault >> optimization options): >> objectaddress.c: In function 'read_objtype_from_string': >> objectaddress.c:2309:9: error: 'type' may be used uninitialized in this >> function [-Werror=maybe-uninitialized] >> return type; > Umm. I think it can only be uninitialized if we fall out of the end of > the array, in which case we're supposed to throw the ERROR and never > return. Is that not working? I do not think you should assume that the compiler is smart enough to deduce that, nor that all compilers even know ereport(ERROR) doesn't return. Personally I don't see the point of the "type" variable at all, anyway. I would have written this as inti; for (i = 0; i < lengthof(ObjectTypeMap); i++) { if (strcmp(ObjectTypeMap[i].tm_name, objtype) == 0) return ObjectTypeMap[i].tm_type; } ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("unrecognized object type \"%s\"", objtype))); return -1;/* keep compiler quiet */ regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] compiler warning read_objtype_from_string()
Peter Eisentraut wrote: > I'm getting the following compiler warning (using nondefault > optimization options): > > objectaddress.c: In function 'read_objtype_from_string': > objectaddress.c:2309:9: error: 'type' may be used uninitialized in this > function [-Werror=maybe-uninitialized] > return type; Umm. I think it can only be uninitialized if we fall out of the end of the array, in which case we're supposed to throw the ERROR and never return. Is that not working? > The comment for the function says > > * Return ObjectType for the given object type as given by > * getObjectTypeDescription; if no valid ObjectType code exists, but it's a > * possible output type from getObjectTypeDescription, return -1. > > But the claim that it can return -1 does not seem supported by the code. Actually, it is -- but the -1 value comes from the ObjectType array. Perhaps the comment should state that explicitely. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] compiler warning read_objtype_from_string()
I'm getting the following compiler warning (using nondefault optimization options): objectaddress.c: In function 'read_objtype_from_string': objectaddress.c:2309:9: error: 'type' may be used uninitialized in this function [-Werror=maybe-uninitialized] return type; The comment for the function says * Return ObjectType for the given object type as given by * getObjectTypeDescription; if no valid ObjectType code exists, but it's a * possible output type from getObjectTypeDescription, return -1. But the claim that it can return -1 does not seem supported by the code. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Compiler warning in pg_am changes
David Rowley writes: > I've attached a small patch to fix new compiler warning which is new as of > 65c5fcd3 Pushed, thanks. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Compiler warning in pg_am changes
Hi, I've attached a small patch to fix new compiler warning which is new as of 65c5fcd3 -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services amcostestimate_cast.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Compiler warning
Peter Geoghegan writes: > Commit 7e2a18a9 must have caused this compiler warning which I now see > on the master branch with my standard release build settings: [ scratches head... ] Dunno why my compiler didn't complain about that. Will fix it in a bit. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Compiler warning
Commit 7e2a18a9 must have caused this compiler warning which I now see on the master branch with my standard release build settings: gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -O2 -g3 -gdwarf-4 -Werror -I. -I../../../../src/include -D_FORTIFY_SOURCE=2 -D_GNU_SOURCE -I/usr/include/libxml2 -c -o logicalfuncs.o logicalfuncs.c -MMD -MP -MF .deps/logicalfuncs.Po postmaster.c: In function ‘ServerLoop’: postmaster.c:1777:9: error: ‘now’ may be used uninitialized in this function [-Werror=maybe-uninitialized] (now - AbortStartTime) >= SIGKILL_CHILDREN_AFTER_SECS) ^ cc1: all warnings being treated as errors make[3]: *** [postmaster.o] Error 1 -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Compiler warning about overflow in xlog.c
Petr Jelinek writes: > my compiler complains about overflow in xlog.c. Yeah, Peter E. reported this yesterday. Since Heikki didn't do anything about that yet, I pushed your fix. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Compiler warning about overflow in xlog.c
Hi, my compiler complains about overflow in xlog.c. There is variable defined as char partialfname[MAXFNAMELEN]; but is used as snprintf(partialfname, MAXPGPATH, "%s.partial", origfname); There is no practical issue as the actual filename length is never over MAXFNAMELEN even with the .partial suffix but the code should still be fixed which is what attached one-line patch does. -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services xlog-overflow-fix.diff Description: binary/octet-stream -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Compiler warning in master branch
David Rowley wrote:> On Mon, Nov 10, 2014 at 4:31 PM, Peter Geoghegan wrote: >> I see this when I build at -O2 from master's tip: >> brin_revmap.c: In function ‘brinRevmapExtend’: >> brin_revmap.c:113:14: error: variable ‘mapBlk’ set but not used >> [-Werror=unused-but-set-variable] >> BlockNumber mapBlk; >> ^ > It would appear just to need the attached. Confirmed and pushed. Thanks to both of you! -- Kevin Grittner EDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Compiler warning in master branch
On Mon, Nov 10, 2014 at 4:31 PM, Peter Geoghegan wrote: > I see this when I build at -O2 from master's tip: > > gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith > -Wdeclaration-after-statement -Wendif-labels > -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing > -fwrapv -fexcess-precision=standard -Werror -I../../../../src/include > -D_FORTIFY_SOURCE=2 -D_GNU_SOURCE -I/usr/include/libxml2 -c -o > brin_revmap.o brin_revmap.c -MMD -MP -MF .deps/brin_revmap.Po > brin_revmap.c: In function ‘brinRevmapExtend’: > brin_revmap.c:113:14: error: variable ‘mapBlk’ set but not used > [-Werror=unused-but-set-variable] > BlockNumber mapBlk; > ^ > > I'm using gcc 4.8.2 here. > > It would appear just to need the attached. Regards David Rowley diff --git a/src/backend/access/brin/brin_revmap.c b/src/backend/access/brin/brin_revmap.c index 7f55ded..272c74e 100644 --- a/src/backend/access/brin/brin_revmap.c +++ b/src/backend/access/brin/brin_revmap.c @@ -110,7 +110,7 @@ brinRevmapTerminate(BrinRevmap *revmap) void brinRevmapExtend(BrinRevmap *revmap, BlockNumber heapBlk) { - BlockNumber mapBlk; + BlockNumber mapBlk PG_USED_FOR_ASSERTS_ONLY; mapBlk = revmap_extend_and_get_blkno(revmap, heapBlk); -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Compiler warning in master branch
I see this when I build at -O2 from master's tip: gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -Werror -I../../../../src/include -D_FORTIFY_SOURCE=2 -D_GNU_SOURCE -I/usr/include/libxml2 -c -o brin_revmap.o brin_revmap.c -MMD -MP -MF .deps/brin_revmap.Po brin_revmap.c: In function ‘brinRevmapExtend’: brin_revmap.c:113:14: error: variable ‘mapBlk’ set but not used [-Werror=unused-but-set-variable] BlockNumber mapBlk; ^ I'm using gcc 4.8.2 here. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] compiler warning in UtfToLocal and LocalToUtf (conv.c)
W dniu 19.07.2013 02:42, Tom Lane pisze: > Hm, what compiler version are you using? I've never seen such a > warning (and that code hasn't changed in some time). gcc version 4.8.1 20130612 (Red Hat 4.8.1-2) (GCC) Regards, Karol -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] compiler warning in UtfToLocal and LocalToUtf (conv.c)
Karol Trzcionka writes: > in the current master head (4cbe3ac3e86790d05c569de4585e5075a62a9b41), > I've noticed the compiler warnings in src/backend/utils/mb/conv.c Hm, what compiler version are you using? I've never seen such a warning (and that code hasn't changed in some time). I agree the code could stand to be fixed, but I'm just curious as to why this is only being seen now. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] compiler warning in UtfToLocal and LocalToUtf (conv.c)
Hello, in the current master head (4cbe3ac3e86790d05c569de4585e5075a62a9b41), I've noticed the compiler warnings in src/backend/utils/mb/conv.c conv.c: In function ‘UtfToLocal’: conv.c:252:23: error: ‘iutf’ may be used uninitialized in this function [-Werror=maybe-uninitialized] ... conv.c: In function ‘LocalToUtf’: conv.c:301:23: error: ‘iiso’ may be used uninitialized in this function [-Werror=maybe-uninitialized] ... The compiler doesn't know that the 'l' may varies between 1 and 4. Hot fix may be: 1. preinitialize it 2. delete last if statement (change else-if to else) 3. change it to switch-case and set default behaviour Regards, Karol -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] compiler warning
Magnus Hagander wrote: > On Thu, Feb 3, 2011 at 04:40, Bruce Momjian wrote: > > I am seeing the following compiler warning for the past few days: > > > > ? ? ? ?basebackup.c:213: warning: variable `ptr' might be clobbered by > > ? ? ? ?`longjmp' or `vfork' > > > > and I see this comment in the file: > > > > ? ? ? ?/* > > ? ? ? ? * Actually do a base backup for the specified tablespaces. > > ? ? ? ? * > > ? ? ? ? * This is split out mainly to avoid complaints about "variable > > might be > > ? ? ? ? * clobbered by longjmp" from stupider versions of gcc. > > ? ? ? ? */ > > > > Seems that isn't working as expected. ?I am using: > > > > ? ? ? ?gcc version 2.95.3 20010315 (release) > > > > with -O1. > > This is the same warning Tom fixed earlier. I have no idea what > actually causes it :( > > I think it's somehow caused by the PG_ENSURE_ERROR_CLEANUP stuff. > Does it go away if you break everything inside the if > (opt->includewal) into it's own function? (Or at least everything > except the pq_putemptymessage() call, because moving that would make > the code very confusing) I added the attached C comment so we know why the warning is generated. We can remove it later if we want, but I want to have it if we get reports about 9.1 problems. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c index b5cda50..d94b61f 100644 *** a/src/backend/replication/basebackup.c --- b/src/backend/replication/basebackup.c *** perform_base_backup(basebackup_options * *** 217,222 --- 217,228 ptr.xlogid = logid; ptr.xrecoff = logseg * XLogSegSize + TAR_SEND_SIZE * i; + /* + * Some old compilers, e.g. 2.95.3/x86, think that passing + * a struct in the same function as a longjump might clobber + * a variable. bjm 2011-02-04 + * http://lists.apple.com/archives/xcode-users/2003/Dec//msg00051.html + */ XLogRead(buf, ptr, TAR_SEND_SIZE); if (pq_putmessage('d', buf, TAR_SEND_SIZE)) ereport(ERROR, -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] compiler warning
On Thu, Feb 3, 2011 at 04:40, Bruce Momjian wrote: > I am seeing the following compiler warning for the past few days: > > basebackup.c:213: warning: variable `ptr' might be clobbered by > `longjmp' or `vfork' > > and I see this comment in the file: > > /* > * Actually do a base backup for the specified tablespaces. > * > * This is split out mainly to avoid complaints about "variable might > be > * clobbered by longjmp" from stupider versions of gcc. > */ > > Seems that isn't working as expected. I am using: > > gcc version 2.95.3 20010315 (release) > > with -O1. This is the same warning Tom fixed earlier. I have no idea what actually causes it :( I think it's somehow caused by the PG_ENSURE_ERROR_CLEANUP stuff. Does it go away if you break everything inside the if (opt->includewal) into it's own function? (Or at least everything except the pq_putemptymessage() call, because moving that would make the code very confusing) -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] compiler warning
I am seeing the following compiler warning for the past few days: basebackup.c:213: warning: variable `ptr' might be clobbered by `longjmp' or `vfork' and I see this comment in the file: /* * Actually do a base backup for the specified tablespaces. * * This is split out mainly to avoid complaints about "variable might be * clobbered by longjmp" from stupider versions of gcc. */ Seems that isn't working as expected. I am using: gcc version 2.95.3 20010315 (release) with -O1. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] compiler warning
Jeff Davis writes: > A little while ago, I saw a compiler warning creep in: > In file included from gram.y:11202: > scan.c: In function âyy_try_NUL_transâ: > scan.c:15765: warning: unused variable âyygâ Yeah, this is a bogosity in the current release of "flex". http://sourceforge.net/tracker/index.php?func=detail&aid=2820457&group_id=97492&atid=618177 regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] compiler warning
A little while ago, I saw a compiler warning creep in: In file included from gram.y:11202: scan.c: In function ‘yy_try_NUL_trans’: scan.c:15765: warning: unused variable ‘yyg’ I know there were some changes to minimum versions, but I think I have those versions (listed below). I don't know if others are seeing this warning or not. Regards, Jeff Davis $ gcc --version gcc (Debian 4.3.3-14) 4.3.3 Copyright (C) 2008 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. $ flex --version flex 2.5.35 $ bison --version bison (GNU Bison) 2.4.1 Written by Robert Corbett and Richard Stallman. Copyright (C) 2008 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Compiler warning cleanup - unitilized const variables, pointer type mismatch
Zdenek Kotala writes: > Tom Lane pÃÅ¡e v Ät 28. 05. 2009 v 11:42 -0400: >> The proposed signature change on psql_completion >> is going to replace a warning on your system with outright failures on >> other people's. > I check readline and definition is still same at least from 5.0 version. > I'm not still understand why it should failure on other systems. I > looked on revision 1.30 of the file and there is only readline-4.2 > support mentioned. Is readline 4.2 the problem? [ pokes around... ] Actually I think the reason it's like this is that very old versions of readline have completion_matches() rather than rl_completion_matches(), and the former is declared to take char * not const char *. So it still would compile, you'd just get cast-away-const warnings. Which is probably okay considering that hardly anyone is likely to still be using such old readline libs anyway. We could try experimenting with that after we branch for 8.5. I'm not eager to fool with it in late beta, as we'd be invalidating any port testing already done. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Compiler warning cleanup - unitilized const variables, pointer type mismatch
Tom Lane píše v čt 28. 05. 2009 v 11:57 -0400: > ). > > AFAICS, Sun's compiler is just too stupid and shouldn't be emitting > this warning. Perhaps the right response is to file a bug report > against the compiler. I checked it and it is already know bug. It is new lint style check in Sun Studio 12. Unfortunately, problem is that current workflow does not allow to detect if code is dead or not in the verification phase. Next sun studio release could fix it. Zdenek -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Compiler warning cleanup - unitilized const variables, pointer type mismatch
Tom Lane píše v čt 28. 05. 2009 v 11:42 -0400: > Zdenek Kotala writes: > > I attached another cleanup patch which fixes following warnings reported > > by Sun Studio: > > I'm not too impressed with any of these. The proposed added > initializers just increase future maintenance effort without solving > any real problem (since the variables are required by C standard to > initialize to zero). Agree. > The proposed signature change on psql_completion > is going to replace a warning on your system with outright failures on > other people's. I check readline and definition is still same at least from 5.0 version. I'm not still understand why it should failure on other systems. I looked on revision 1.30 of the file and there is only readline-4.2 support mentioned. Is readline 4.2 the problem? Zdenek -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Compiler warning cleanup - unitilized const variables, pointer type mismatch
Michael Meskes writes: > I have to admit that those version look strikingly unsimilar to me. There is > no > reference to Rhs[N] in our macro at all. But then I have no idea whether this > is needed. The default version of the macro is intended to track both the starting and ending locations of every construct. Our simplified version only tracks the starting locations. The inputs are RHS[k], the location values for the constituent elements of the current production, and the output is the location value for the construct being formed. In the default version, you naturally want to copy the start of RHS[1] and the end of RHS[N], where N is the number of production elements. In ours, we just copy the location of RHS[1]. However, both macros need a special case for empty productions (with N = 0). AFAICS, Sun's compiler is just too stupid and shouldn't be emitting this warning. Perhaps the right response is to file a bug report against the compiler. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Compiler warning cleanup - unitilized const variables, pointer type mismatch
Zdenek Kotala writes: > I attached another cleanup patch which fixes following warnings reported > by Sun Studio: I'm not too impressed with any of these. The proposed added initializers just increase future maintenance effort without solving any real problem (since the variables are required by C standard to initialize to zero). The proposed signature change on psql_completion is going to replace a warning on your system with outright failures on other people's. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Compiler warning cleanup - unitilized const variables, pointer type mismatch
Michael Meskes píše v čt 28. 05. 2009 v 14:47 +0200: > On Thu, May 28, 2009 at 01:51:07PM +0200, Zdenek Kotala wrote: > > Problem is with YYLLOC_DEFAULT. When I look on macro definition > > > > #define YYLLOC_DEFAULT(Current, Rhs, N) \ > > Current.first_line = Rhs[1].first_line; \ > > Current.first_column = Rhs[1].first_column;\ > > Current.last_line= Rhs[N].last_line; \ > > Current.last_column = Rhs[N].last_column; > > > > It seems to me that it is OK, because 1 is used as a index which finally > > point on yyerror_range[0]. > > Wait, this is the bison definition. Well to be more precise the bison > definition in your bison version. Mine is different: I took it from documentation. I have same as you in generated code. > # define YYLLOC_DEFAULT(Current, Rhs, N)\ > do \ > if (YYID (N))\ > { \ > (Current).first_line = YYRHSLOC (Rhs, 1).first_line;\ > (Current).first_column = YYRHSLOC (Rhs, 1).first_column; \ > (Current).last_line= YYRHSLOC (Rhs, N).last_line; \ > (Current).last_column = YYRHSLOC (Rhs, N).last_column; \ > } \ > else \ > { \ > (Current).first_line = (Current).last_line = \ > YYRHSLOC (Rhs, 0).last_line;\ > (Current).first_column = (Current).last_column = \ > YYRHSLOC (Rhs, 0).last_column; \ >} \ > while (YYID (0)) > > Having said that, it doesn't really matter as we redefine the macro: > > #define YYLLOC_DEFAULT(Current, Rhs, N) \ > do { \ > if (N) \ > (Current) = (Rhs)[1]; \ > else \ > (Current) = (Rhs)[0]; \ > } while (0) > > I have to admit that those version look strikingly unsimilar to me. There is > no > reference to Rhs[N] in our macro at all. But then I have no idea whether this > is needed. Current is only int. See gramparse.h. I think we could rewrite it this way: #define YYLLOC_DEFAULT(Current, Rhs, N) \ do { \ if (N) \ (Current) = (Rhs)[1]; \ else \ (Current) = (Rhs)[N]; \ } while (0) It is same result and compiler is quite. Zdenek -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Compiler warning cleanup - unitilized const variables, pointer type mismatch
On Thu, May 28, 2009 at 01:51:07PM +0200, Zdenek Kotala wrote: > Problem is with YYLLOC_DEFAULT. When I look on macro definition > > #define YYLLOC_DEFAULT(Current, Rhs, N) \ > Current.first_line = Rhs[1].first_line; \ > Current.first_column = Rhs[1].first_column;\ > Current.last_line= Rhs[N].last_line; \ > Current.last_column = Rhs[N].last_column; > > It seems to me that it is OK, because 1 is used as a index which finally > point on yyerror_range[0]. Wait, this is the bison definition. Well to be more precise the bison definition in your bison version. Mine is different: # define YYLLOC_DEFAULT(Current, Rhs, N)\ do \ if (YYID (N))\ { \ (Current).first_line = YYRHSLOC (Rhs, 1).first_line;\ (Current).first_column = YYRHSLOC (Rhs, 1).first_column; \ (Current).last_line= YYRHSLOC (Rhs, N).last_line; \ (Current).last_column = YYRHSLOC (Rhs, N).last_column; \ } \ else \ { \ (Current).first_line = (Current).last_line = \ YYRHSLOC (Rhs, 0).last_line;\ (Current).first_column = (Current).last_column = \ YYRHSLOC (Rhs, 0).last_column; \ } \ while (YYID (0)) Having said that, it doesn't really matter as we redefine the macro: #define YYLLOC_DEFAULT(Current, Rhs, N) \ do { \ if (N) \ (Current) = (Rhs)[1]; \ else \ (Current) = (Rhs)[0]; \ } while (0) I have to admit that those version look strikingly unsimilar to me. There is no reference to Rhs[N] in our macro at all. But then I have no idea whether this is needed. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org ICQ: 179140304, AIM/Yahoo: michaelmeskes, Jabber: mes...@jabber.org Go VfL Borussia! Go SF 49ers! Use Debian GNU/Linux! Use PostgreSQL! -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Compiler warning cleanup - unitilized const variables, pointer type mismatch
Michael Meskes píše v čt 28. 05. 2009 v 13:33 +0200: > On Thu, May 28, 2009 at 11:11:20AM +0200, Zdenek Kotala wrote: > > I attached another cleanup patch which fixes following warnings reported > > by Sun Studio: > > ... > > "preproc.c", line 39569: warning: pointer expression or its operand do not > > point to the same object yyerror_range, result is undefined and > > non-portable > > ... > > Following list is still unfixed plus see my comments: > > > > "gram.c", line 28487: warning: pointer expression or its operand do not > > point to the same object yyerror_range, result is undefined and > > non-portable > > ... > > These two should be the same, both coming from bison. Both files are > auto-generated, thus it might be bison that has to be fixed to remove this > warning. yeah it is generated, but question is if generated code is valid or it is bug in bison. If it bison bug we need to care about it. There is the code: yyerror_range[1] = yylloc; /* Using YYLLOC is tempting, but would change the location of the look-ahead. YYLOC is available though. */ YYLLOC_DEFAULT (yyloc, (yyerror_range - 1), 2); *++yylsp = yyloc; Problem is with YYLLOC_DEFAULT. When I look on macro definition #define YYLLOC_DEFAULT(Current, Rhs, N) \ Current.first_line = Rhs[1].first_line; \ Current.first_column = Rhs[1].first_column;\ Current.last_line= Rhs[N].last_line; \ Current.last_column = Rhs[N].last_column; It seems to me that it is OK, because 1 is used as a index which finally point on yyerror_range[0]. > Given that I didn't find any mentioning of preproc in your patch I > suppose it just hit the wrong list though. I'm sorry copy paste error. Yeah, I did not fix preproc too. Zdenek -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Compiler warning cleanup - unitilized const variables, pointer type mismatch
On Thu, May 28, 2009 at 11:11:20AM +0200, Zdenek Kotala wrote: > I attached another cleanup patch which fixes following warnings reported > by Sun Studio: > ... > "preproc.c", line 39569: warning: pointer expression or its operand do not > point to the same object yyerror_range, result is undefined and non-portable > ... > Following list is still unfixed plus see my comments: > > "gram.c", line 28487: warning: pointer expression or its operand do not point > to the same object yyerror_range, result is undefined and non-portable > ... These two should be the same, both coming from bison. Both files are auto-generated, thus it might be bison that has to be fixed to remove this warning. Given that I didn't find any mentioning of preproc in your patch I suppose it just hit the wrong list though. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org ICQ: 179140304, AIM/Yahoo: michaelmeskes, Jabber: mes...@jabber.org Go VfL Borussia! Go SF 49ers! Use Debian GNU/Linux! Use PostgreSQL! -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Compiler warning cleanup - unitilized const variables, pointer type mismatch
I attached another cleanup patch which fixes following warnings reported by Sun Studio: "zic.c", line 1534: warning: const object should have initializer: tzh0 "dynloader.c", line 7: warning: empty translation unit "pgstat.c", line 666: warning: const object should have initializer: all_zeroes "pgstat.c", line 799: warning: const object should have initializer: all_zeroes "pgstat.c", line 2552: warning: const object should have initializer: all_zeroes "preproc.c", line 39569: warning: pointer expression or its operand do not point to the same object yyerror_range, result is undefined and non-portable "tab-complete.c", line 587: warning: assignment type mismatch: pointer to function(pointer to const char, int, int) returning pointer to pointer to char "=" pointer to void Following list is still unfixed plus see my comments: "gram.c", line 28487: warning: pointer expression or its operand do not point to the same object yyerror_range, result is undefined and non-portable - This is really strange warning. The code is really strange because it points to -1 index of array, but I'm not bison guru. Maybe it is correct, but it would be good if somebody check it. "../../../src/include/pg_config.h", line 782: warning: macro redefined: _FILE_OFFSET_BITS - This probably needs some extra love in configure. "regc_lex.c", line 401: warning: loop not entered at top "regc_lex.c", line 484: warning: loop not entered at top "regc_lex.c", line 578: warning: loop not entered at top "regc_lex.c", line 610: warning: loop not entered at top "regc_lex.c", line 870: warning: loop not entered at top "regc_lex.c", line 1073: warning: loop not entered at top "postgres.c", line 3845: warning: loop not entered at top - Assert on not reached place probably confuse compiler. I'm not sure if it make sense nowadays? Most compiler should optimize this anyway and code is removed. I suppose to remove these asserts. Zdenek diff -Nrc pgsql.orig.8fc4f032818a/src/backend/port/dynloader/solaris.c pgsql.orig/src/backend/port/dynloader/solaris.c *** pgsql.orig.8fc4f032818a/src/backend/port/dynloader/solaris.c 2009-05-28 11:09:24.874020865 +0200 --- pgsql.orig/src/backend/port/dynloader/solaris.c 2009-05-28 11:09:24.893688008 +0200 *** *** 5,7 --- 5,11 * * see solaris.h */ + + /* compiler complains about empty unit, let compiler quite */ + extern int no_such_variable; + diff -Nrc pgsql.orig.8fc4f032818a/src/backend/postmaster/pgstat.c pgsql.orig/src/backend/postmaster/pgstat.c *** pgsql.orig.8fc4f032818a/src/backend/postmaster/pgstat.c 2009-05-28 11:09:24.882883806 +0200 --- pgsql.orig/src/backend/postmaster/pgstat.c 2009-05-28 11:09:24.894106321 +0200 *** *** 662,669 void pgstat_report_stat(bool force) { ! /* we assume this inits to all zeroes: */ ! static const PgStat_TableCounts all_zeroes; static TimestampTz last_report = 0; TimestampTz now; --- 662,668 void pgstat_report_stat(bool force) { ! static const PgStat_TableCounts all_zeroes = {0,0,0,0,0,0,0,0,0,0,0}; static TimestampTz last_report = 0; TimestampTz now; *** *** 795,802 static void pgstat_send_funcstats(void) { ! /* we assume this inits to all zeroes: */ ! static const PgStat_FunctionCounts all_zeroes; PgStat_MsgFuncstat msg; PgStat_BackendFunctionEntry *entry; --- 794,800 static void pgstat_send_funcstats(void) { ! static const PgStat_FunctionCounts all_zeroes = {0,{0,0},{0,0}}; PgStat_MsgFuncstat msg; PgStat_BackendFunctionEntry *entry; *** *** 2548,2555 void pgstat_send_bgwriter(void) { ! /* We assume this initializes to zeroes */ ! static const PgStat_MsgBgWriter all_zeroes; /* * This function can be called even if nothing at all has happened. In --- 2546,2552 void pgstat_send_bgwriter(void) { ! static const PgStat_MsgBgWriter all_zeroes = { {0,0},0,0,0,0,0,0,0}; /* * This function can be called even if nothing at all has happened. In diff -Nrc pgsql.orig.8fc4f032818a/src/bin/psql/tab-complete.c pgsql.orig/src/bin/psql/tab-complete.c *** pgsql.orig.8fc4f032818a/src/bin/psql/tab-complete.c 2009-05-28 11:09:24.890322342 +0200 --- pgsql.orig/src/bin/psql/tab-complete.c 2009-05-28 11:09:24.894581639 +0200 *** *** 557,563 /* Forward declaration of functions */ ! static char **psql_completion(char *text, int start, int end); static char *create_command_generator(const char *text, int state); static char *drop_command_generator(const char *text, int state); static char *complete_from_query(const char *text, int state); --- 557,563 /* Forward declaration of functions */ ! static char **psql_completion(const char *text, int start, int end); static char *create_command_generator(const char *text, int state); static char *drop_command_generator(const cha
Re: [HACKERS] Compiler warning
On Thursday 21 May 2009 14:29:51 Tom Lane wrote: > Peter Eisentraut writes: > > On Thursday 21 May 2009 10:01:59 Heikki Linnakangas wrote: > >> Hmm, the patch looks fine to me. The strings are marked with > >> gettext_noop() in the array, and passed through _() when used in errmsg. > > > > But his patch changes that to > > > > errhint("%s", _(wentry->drophint_msg)) > > > > so it ends up being run through gettext twice. > > How so ? errhint only passes its first argument through gettext. Yeah, not thinking clearly. So I guess this patch is OK if people care about that warning. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Compiler warning
Peter Eisentraut writes: > On Thursday 21 May 2009 10:01:59 Heikki Linnakangas wrote: >> Hmm, the patch looks fine to me. The strings are marked with >> gettext_noop() in the array, and passed through _() when used in errmsg. > But his patch changes that to > errhint("%s", _(wentry->drophint_msg)) > so it ends up being run through gettext twice. How so ? errhint only passes its first argument through gettext. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Compiler warning
On Thursday 21 May 2009 10:01:59 Heikki Linnakangas wrote: > Peter Eisentraut wrote: > > Note that applying this patch would introduce a double-translation issue > > of the sort that you had complained about a while ago. I'm unsure which > > way to proceed here. > > Hmm, the patch looks fine to me. The strings are marked with > gettext_noop() in the array, and passed through _() when used in errmsg. But his patch changes that to errhint("%s", _(wentry->drophint_msg)) so it ends up being run through gettext twice. Which has recently been raised as a concern. Both of these competing issues -- the compiler warning and double translation -- appear to be minor in practice, but we apparently have to make a choice which one we plan to fix now and in the future. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Compiler warning
Peter Eisentraut wrote: Note that applying this patch would introduce a double-translation issue of the sort that you had complained about a while ago. I'm unsure which way to proceed here. Hmm, the patch looks fine to me. The strings are marked with gettext_noop() in the array, and passed through _() when used in errmsg. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Compiler warning
On Wednesday 20 May 2009 16:24:21 Tom Lane wrote: > Heikki Linnakangas writes: > > Hmm, it is a false alarm, but would be nice to have a warning-free > > build. I don't see that warning here on gcc 4.3.3 by default, but I do > > when I set -Wformat-security. I presume you had that set as well. > > Would it be worth having configure probe for that switch and add it to > CFLAGS if available? Note that applying this patch would introduce a double-translation issue of the sort that you had complained about a while ago. I'm unsure which way to proceed here. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Compiler warning
Heikki Linnakangas writes: > Hmm, it is a false alarm, but would be nice to have a warning-free > build. I don't see that warning here on gcc 4.3.3 by default, but I do > when I set -Wformat-security. I presume you had that set as well. Would it be worth having configure probe for that switch and add it to CFLAGS if available? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Compiler warning
Fujii Masao wrote: I encountered the following compiler warning in 8.4dev. Attached is the patch to fix this problem. Is this worth committing? -- tablecmds.c: In function 'DropErrorMsgWrongType': tablecmds.c:606: warning: format not a string literal and no format arguments -- Hmm, it is a false alarm, but would be nice to have a warning-free build. I don't see that warning here on gcc 4.3.3 by default, but I do when I set -Wformat-security. I presume you had that set as well. Applied. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Compiler warning
Hi, I encountered the following compiler warning in 8.4dev. Attached is the patch to fix this problem. Is this worth committing? -- tablecmds.c: In function 'DropErrorMsgWrongType': tablecmds.c:606: warning: format not a string literal and no format arguments -- $ gcc -v Using built-in specs. Target: i486-linux-gnu Configured with: ../src/configure -v --with-pkgversion='Ubuntu 4.3.3-5ubuntu4' --with-bugurl=file:///usr/share/doc/gcc-4.3/README.Bugs --enable-languages=c,c++,fortran,objc,obj-c++ --prefix=/usr --enable-shared --with-system-zlib --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --enable-nls --with-gxx-include-dir=/usr/include/c++/4.3 --program-suffix=-4.3 --enable-clocale=gnu --enable-libstdcxx-debug --enable-objc-gc --enable-mpfr --enable-targets=all --with-tune=generic --enable-checking=release --build=i486-linux-gnu --host=i486-linux-gnu --target=i486-linux-gnu Thread model: posix gcc version 4.3.3 (Ubuntu 4.3.3-5ubuntu4) Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center Index: src/backend/commands/tablecmds.c === RCS file: /projects/cvsroot/pgsql/src/backend/commands/tablecmds.c,v retrieving revision 1.284 diff -r1.284 tablecmds.c 609c609 < (wentry->kind != '\0') ? errhint(wentry->drophint_msg) : 0)); --- > (wentry->kind != '\0') ? errhint("%s", _(wentry->drophint_msg)) : 0)); -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Compiler warning with 'fast' variable
Tom Lane wrote: > Bruce Momjian writes: > > Tom Lane wrote: > >> We could stick a volatile on it but I'd like to find out why this > >> particular variable seems to need that. > > > You ready for this; changing 'bool' to 'int' supressed the warning: > > int fast = PG_GETARG_BOOL(1); > > Well, that's a compiler bug :-(. Probably platform-specific, too, > which is why I don't see it on HPPA. > > I think that the above variant is the least ugly of the alternatives > you show as working, and definitely less ugly than plastering volatile > on it. Well, let's leave it alone and see if anyone else find it; I can mask it on my end. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Compiler warning with 'fast' variable
Bruce Momjian writes: > Tom Lane wrote: >> We could stick a volatile on it but I'd like to find out why this >> particular variable seems to need that. > You ready for this; changing 'bool' to 'int' supressed the warning: > int fast = PG_GETARG_BOOL(1); Well, that's a compiler bug :-(. Probably platform-specific, too, which is why I don't see it on HPPA. I think that the above variant is the least ugly of the alternatives you show as working, and definitely less ugly than plastering volatile on it. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Compiler warning with 'fast' variable
Tom Lane wrote: > Bruce Momjian writes: > > Any idea why I am seeing this warning with the new pg_start_backup() > > 'fast' flag? > > > xlog.c:6917: warning: variable `fast' might be clobbered by > > `longjmp' or `vfork' > > > The line is in a PG_ENSURE_ERROR_CLEANUP() block. This is with gcc > > version 2.95.3. > > That's pretty bizarre --- I don't see it here with gcc 2.95.3, > and there is no reason for such a warning to appear on a variable > that isn't changed during the function. > > We could stick a volatile on it but I'd like to find out why this > particular variable seems to need that. You ready for this; changing 'bool' to 'int' supressed the warning: int fast = PG_GETARG_BOOL(1); Using 'char' returns the warning. Changing the assignment to 'true' also fixes it: boolfast = true; This also generates no warning about longjmp: boolfast = PG_GETARG_TEXT(1); No warning here either: boolfast = (bool) PG_GETARG_DATUM(0); This generates the warning: boolfast = ((bool) ((bool) (PG_GETARG_DATUM(1)) != 0)); This does not: boolfast = (bool) (PG_GETARG_DATUM(1) != 0); -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Compiler warning with 'fast' variable
Bruce Momjian writes: > Any idea why I am seeing this warning with the new pg_start_backup() > 'fast' flag? > xlog.c:6917: warning: variable `fast' might be clobbered by > `longjmp' or `vfork' > The line is in a PG_ENSURE_ERROR_CLEANUP() block. This is with gcc > version 2.95.3. That's pretty bizarre --- I don't see it here with gcc 2.95.3, and there is no reason for such a warning to appear on a variable that isn't changed during the function. We could stick a volatile on it but I'd like to find out why this particular variable seems to need that. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Compiler warning with 'fast' variable
Any idea why I am seeing this warning with the new pg_start_backup() 'fast' flag? xlog.c:6917: warning: variable `fast' might be clobbered by `longjmp' or `vfork' The line is in a PG_ENSURE_ERROR_CLEANUP() block. This is with gcc version 2.95.3. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Compiler warning in ecpglib/execute.c
On Tue, Feb 03, 2009 at 04:06:39PM -0300, Alvaro Herrera wrote: > Note that spoonbill is still red. This animal is special because it > uses some peculiar malloc flags. Not just spoonbill, five of them are. Without access to a failing machine this is difficult to debug. I found some missing error checks in that library that might explain the segfaults, but I still don't see why the the test is failing. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org ICQ: 179140304, AIM/Yahoo: michaelmeskes, Jabber: mes...@jabber.org Go VfL Borussia! Go SF 49ers! Use Debian GNU/Linux! Use PostgreSQL! -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Compiler warning in ecpglib/execute.c
Michael Meskes wrote: > On Mon, Feb 02, 2009 at 02:56:18PM -0500, Tom Lane wrote: > > CVS HEAD is producing > > ... > > Thanks for the note, I missed this copy&paste error of mine. Fixed in HEAD. > This should alos make the buildfarm green again. Note that spoonbill is still red. This animal is special because it uses some peculiar malloc flags. -- Alvaro Herrerahttp://www.advogato.org/person/alvherre "La persona que no quería pecar / estaba obligada a sentarse en duras y empinadas sillas/ desprovistas, por cierto de blandos atenuantes" (Patricio Vogel) -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Compiler warning in ecpglib/execute.c
On Mon, Feb 02, 2009 at 02:56:18PM -0500, Tom Lane wrote: > CVS HEAD is producing > ... Thanks for the note, I missed this copy&paste error of mine. Fixed in HEAD. This should alos make the buildfarm green again. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org ICQ: 179140304, AIM/Yahoo: michaelmeskes, Jabber: mes...@jabber.org Go VfL Borussia! Go SF 49ers! Use Debian GNU/Linux! Use PostgreSQL! -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Compiler warning in ecpglib/execute.c
CVS HEAD is producing execute.c: In function 'ecpg_store_result': execute.c:394: warning: 'act_tuple' may be used uninitialized in this function It looks to me like this is an actual bug, not just the compiler being insufficiently smart to prove the variable is set before use. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Compiler warning (V7.1 plpgsql)
It's nothing but unused definitions. PostgreSQL7.1 compiles and works for me. I got following warnings for PL/PgSQL make[2]: `/opt/rh7/postgresql/postgresql-7.1/src/pl' make[3]: `/opt/rh7/postgresql/postgresql-7.1/src/pl/plpgsql' make -C src all make[4]: `/opt/rh7/postgresql/postgresql-7.1/src/pl/plpgsql/src' gcc -c -I. -I../../../../src/include -I/usr/local/ssl/include -O2 -Wall -Wmissin g-prototypes -Wmissing-declarations -fpic -o pl_parse.o pl_gram.c In file included from gram.y:44: lex.plpgsql_yy.c: In function `plpgsql_yylex': lex.plpgsql_yy.c:972: warning: label `find_rule' defined but not used gram.y: At top level: lex.plpgsql_yy.c:2223: warning: `plpgsql_yy_flex_realloc' defined but not used Hope this helps, -- Yasuo Ohgaki ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])