Re: [HACKERS] compiler warning with VS 2017

[Tom Lane]

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

[Petr Jelinek]

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

[Tom Lane]

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

[Petr Jelinek]

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

[Tom Lane]

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

[Haribabu Kommi]

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