compiler warning with VS 2017

Started by Haribabu Kommialmost 9 years ago6 messageshackers
Jump to latest
#1Haribabu Kommi
kommi.haribabu@gmail.com

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]https://msdn.microsoft.com/en-us/library/h97f4b9y.aspx.

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

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Haribabu Kommi (#1)
Re: compiler warning with VS 2017

Haribabu Kommi <kommi.haribabu@gmail.com> 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

#3Petr Jelinek
petr@2ndquadrant.com
In reply to: Tom Lane (#2)
Re: compiler warning with VS 2017

On 05/05/17 06:50, Tom Lane wrote:

Haribabu Kommi <kommi.haribabu@gmail.com> 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

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Petr Jelinek (#3)
Re: compiler warning with VS 2017

Petr Jelinek <petr.jelinek@2ndquadrant.com> 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

#5Petr Jelinek
petr@2ndquadrant.com
In reply to: Tom Lane (#4)
Re: compiler warning with VS 2017

On 05/05/17 16:56, Tom Lane wrote:

Petr Jelinek <petr.jelinek@2ndquadrant.com> 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

#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Petr Jelinek (#5)
Re: compiler warning with VS 2017

Petr Jelinek <petr.jelinek@2ndquadrant.com> 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