Re: Today's failures on buildfarm member longfin

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