pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedToday's failures on buildfarm member longfin
4+ messages / 3 participants
[nested] [flat]
* Today's failures on buildfarm member longfin
@ 2017-03-28 15:25 Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 2 replies; 4+ messages in thread
From: Tom Lane @ 2017-03-28 15:25 UTC (permalink / raw)
To: pgsql-hackers
Last night I updated longfin's host to the latest macOS and XCode point
releases. That brought with it a new clang version which is slightly
pickier than the old: it's complaining about
log.c:5047:16: error: implicit conversion from 'int' to 'char' changes value from 255 to -1 [-Werror,-Wconstant-conversion]
*(recptr++) = XLR_BLOCK_ID_DATA_SHORT;
~ ^~~~~~~~~~~~~~~~~~~~~~~
../../../../src/include/access/xlogrecord.h:223:34: note: expanded from macro 'XLR_BLOCK_ID_DATA_SHORT'
#define XLR_BLOCK_ID_DATA_SHORT 255
^~~
(Manual investigation reveals there's about 5 of these altogether;
longfin's report stops with the first.)
Aleksander Alekseev complained about this previously in
https://www.postgresql.org/message-id/20170220141239.GD12278@e733.localdomain
That thread went off into the weeds discussing whether or not we wanted
to do something about clang breaking autoconf's test for whether strlcpy
is declared, which we didn't particularly; but we forgot about the other
issues.
Since, per previous agreement[1], longfin is running with -Werror, we
either have to do something about this or revert the decision to make it
use -Werror --- and I'm not too pleased about the latter idea. We should
not have made that agreement if we were going to abandon it at the first
sign of pain.
As noted in the other thread, we could either fix this in a
quick-and-dirty way by casting XLR_BLOCK_ID_DATA_SHORT and related
values to "char" explicitly, or we could run around and change the
target pointer variables to be "unsigned char *". The latter could
prove to be pretty invasive if we try to carry it out fully, while
if we don't, then we're arguably just moving the ugly casts someplace
else.
Opinions?
regards, tom lane
[1] https://www.postgresql.org/message-id/32279.1487355685@sss.pgh.pa.us
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: Today's failures on buildfarm member longfin
@ 2017-03-28 15:27 Robert Haas <robertmhaas@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 0 replies; 4+ messages in thread
From: Robert Haas @ 2017-03-28 15:27 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-hackers
On Tue, Mar 28, 2017 at 11:25 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> As noted in the other thread, we could either fix this in a
> quick-and-dirty way by casting XLR_BLOCK_ID_DATA_SHORT and related
> values to "char" explicitly, or we could run around and change the
> target pointer variables to be "unsigned char *". The latter could
> prove to be pretty invasive if we try to carry it out fully, while
> if we don't, then we're arguably just moving the ugly casts someplace
> else.
>
> Opinions?
signed char sucks. Let's hit it with a stick.
:-)
--
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
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: Today's failures on buildfarm member longfin
@ 2017-03-28 15:31 Andres Freund <andres@anarazel.de>
parent: Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 1 reply; 4+ messages in thread
From: Andres Freund @ 2017-03-28 15:31 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-hackers
On 2017-03-28 11:25:08 -0400, Tom Lane wrote:
> Last night I updated longfin's host to the latest macOS and XCode point
> releases. That brought with it a new clang version which is slightly
> pickier than the old: it's complaining about
>
> log.c:5047:16: error: implicit conversion from 'int' to 'char' changes value from 255 to -1 [-Werror,-Wconstant-conversion]
> *(recptr++) = XLR_BLOCK_ID_DATA_SHORT;
> ~ ^~~~~~~~~~~~~~~~~~~~~~~
> ../../../../src/include/access/xlogrecord.h:223:34: note: expanded from macro 'XLR_BLOCK_ID_DATA_SHORT'
> #define XLR_BLOCK_ID_DATA_SHORT 255
> ^~~
>
> (Manual investigation reveals there's about 5 of these altogether;
> longfin's report stops with the first.)
> Since, per previous agreement[1], longfin is running with -Werror, we
> either have to do something about this or revert the decision to make it
> use -Werror --- and I'm not too pleased about the latter idea. We should
> not have made that agreement if we were going to abandon it at the first
> sign of pain.
If necessary we could do that more targeted with
-Wno-error=constant-conversion, but I think we should just fix this.
> As noted in the other thread, we could either fix this in a
> quick-and-dirty way by casting XLR_BLOCK_ID_DATA_SHORT and related
> values to "char" explicitly, or we could run around and change the
> target pointer variables to be "unsigned char *". The latter could
> prove to be pretty invasive if we try to carry it out fully, while
> if we don't, then we're arguably just moving the ugly casts someplace
> else.
I think both are ok with me. Could also just use memcpy instead of
direct assignment, but that seems too complicated. I'd personally just
go with Aleksander's casts.
Greetings,
Andres Freund
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: Today's failures on buildfarm member longfin
@ 2017-03-28 15:56 Tom Lane <tgl@sss.pgh.pa.us>
parent: Andres Freund <andres@anarazel.de>
0 siblings, 0 replies; 4+ messages in thread
From: Tom Lane @ 2017-03-28 15:56 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: pgsql-hackers
Andres Freund <andres@anarazel.de> writes:
> On 2017-03-28 11:25:08 -0400, Tom Lane wrote:
>> Since, per previous agreement[1], longfin is running with -Werror, we
>> either have to do something about this or revert the decision to make it
>> use -Werror --- and I'm not too pleased about the latter idea. We should
>> not have made that agreement if we were going to abandon it at the first
>> sign of pain.
> If necessary we could do that more targeted with
> -Wno-error=constant-conversion, but I think we should just fix this.
Yeah, that option seems like it would lose important error detection.
>> As noted in the other thread, we could either fix this in a
>> quick-and-dirty way by casting XLR_BLOCK_ID_DATA_SHORT and related
>> values to "char" explicitly, or we could run around and change the
>> target pointer variables to be "unsigned char *".
> I think both are ok with me. Could also just use memcpy instead of
> direct assignment, but that seems too complicated. I'd personally just
> go with Aleksander's casts.
Further investigation says that these warnings now also appear in the
9.5 and 9.6 branches, which were previously warning-clean except for
ye olde unused-variable flex bug. So I'd like a solution that can be
back-patched, which says that the localized casts to char are the way to
go. Maybe somebody else will want to undertake a more general cleanup,
but I don't want to spend time on that right 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
^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2017-03-28 15:56 UTC | newest]
Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2017-03-28 15:25 Today's failures on buildfarm member longfin Tom Lane <tgl@sss.pgh.pa.us>
2017-03-28 15:27 ` Robert Haas <robertmhaas@gmail.com>
2017-03-28 15:31 ` Andres Freund <andres@anarazel.de>
2017-03-28 15:56 ` Tom Lane <tgl@sss.pgh.pa.us>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox