Re: warning in code while building on windows

From: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Andrew Dunstan <andrew(at)dunslane(dot)net>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: warning in code while building on windows
Date: 2013-08-19 05:06:37
Message-ID: CAA4eK1K_ZJxKjVhEhhWoLVK1oMM1oYt2gNSf9WmLw9XVrj7rqg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Aug 19, 2013 at 9:33 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>>> Amit Kapila escribió:
>>>> 1>.\src\backend\utils\cache\relfilenodemap.c(213) : warning C4101:
>>>> 'isnull' : unreferenced local variable
>
>> The macro is pretty gcc-specific, isn't it?
>
> If that's the problem, why isn't Amit seeing a boatload of similar
> warnings elsewhere?

If I try to change other place similar to relfilenodemap.c, I am
getting similar warning. For example:

Original Code
--------------------
ExecMaterial()
{
..
/*
* Allocate a second read pointer to serve as the mark. We know it
* must have index 1, so needn't store that.
*/
int ptrno PG_USED_FOR_ASSERTS_ONLY;

ptrno = tuplestore_alloc_read_pointer(tuplestorestate,
node->eflags);
Assert(ptrno == 1);
..
}

Modified Code
---------------------
ExecMaterial()
{
..
/*
* Allocate a second read pointer to serve as the mark. We know it
* must have index 1, so needn't store that.
*/
int ptrno PG_USED_FOR_ASSERTS_ONLY;

#ifdef USE_ASSERT_CHECKING

ptrno = tuplestore_alloc_read_pointer(tuplestorestate,
node->eflags);
Assert(ptrno == 1);
#endif
..
}

After above modification it gives below compilation error:
1>.\src\backend\executor\nodeMaterial.c(69) : warning C4101: 'ptrno' :
unreferenced local variable

Coming to original warning, changing the code as below removes warning:

RelidByRelfilenode()
{
..
#ifdef USE_ASSERT_CHECKING
if (assert_enabled)
{
Oid check;
bool isnull PG_USED_FOR_ASSERTS_ONLY;
check = fastgetattr(ntp, Anum_pg_class_reltablespace,
RelationGetDescr(relation),
&isnull);
Assert(!isnull && check == reltablespace);

check = fastgetattr(ntp, Anum_pg_class_relfilenode,
RelationGetDescr(relation),
&isnull);
Assert(!isnull && check == relfilenode);
}
#endif

Moving isnull declaration inside if block resolves current compilation
warning, I think in any case it is better to declare inside if block
as it is used in that block only.

I think resolving the bigger problem such that it should not give
warning for such usage in MSVC is important, but can be dealt as a
separate thread/patch.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Heikki Linnakangas 2013-08-19 08:20:42 Re: Should we remove "not fast" promotion at all?
Previous Message Etsuro Fujita 2013-08-19 04:38:13 Re: 9.3 release notes suggestions