Thread: warning in code while building on windows
1>.\src\backend\utils\cache\relfilenodemap.c(213) : warning C4101: 'isnull' : unreferenced local variable It seems this variable is used only under macro USE_ASSERT_CHECKING, so it is better to declare under this macro only. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
Amit Kapila escribió: > 1>.\src\backend\utils\cache\relfilenodemap.c(213) : warning C4101: > 'isnull' : unreferenced local variable > > It seems this variable is used only under macro USE_ASSERT_CHECKING, > so it is better to declare under this macro only. We have a macro for this, PG_USED_FOR_ASSERTS_ONLY. This should silence it; if the msvc stuff is not picking it up then maybe we need to tweak the definition of the macro somehow. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
On 08/18/2013 05:08 PM, Alvaro Herrera wrote: > Amit Kapila escribió: >> 1>.\src\backend\utils\cache\relfilenodemap.c(213) : warning C4101: >> 'isnull' : unreferenced local variable >> >> It seems this variable is used only under macro USE_ASSERT_CHECKING, >> so it is better to declare under this macro only. > We have a macro for this, PG_USED_FOR_ASSERTS_ONLY. This should silence > it; if the msvc stuff is not picking it up then maybe we need to tweak > the definition of the macro somehow. > The macro is pretty gcc-specific, isn't it? For MSVC we would probably need to surround the declaration with something like these lines. #pragma warning(disable:4101) #pragma warning(default:4101) cheers andrew
Andrew Dunstan escribió: > > On 08/18/2013 05:08 PM, Alvaro Herrera wrote: > >Amit Kapila escribió: > >>1>.\src\backend\utils\cache\relfilenodemap.c(213) : warning C4101: > >>'isnull' : unreferenced local variable > >> > >>It seems this variable is used only under macro USE_ASSERT_CHECKING, > >>so it is better to declare under this macro only. > >We have a macro for this, PG_USED_FOR_ASSERTS_ONLY. This should silence > >it; if the msvc stuff is not picking it up then maybe we need to tweak > >the definition of the macro somehow. > > The macro is pretty gcc-specific, isn't it? > > For MSVC we would probably need to surround the declaration with > something like these lines. > > #pragma warning(disable:4101) > #pragma warning(default:4101) That seems pretty difficult to do with the position we've chosen for the macro. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Andrew Dunstan <andrew@dunslane.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? regards, tom lane
On Mon, Aug 19, 2013 at 9:33 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Andrew Dunstan <andrew@dunslane.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'tstore 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'tstore 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