On Wed, Mar 11, 2015 at 11:30 PM, Michael Paquier
<[email protected]> wrote:
> On Wed, Mar 11, 2015 at 11:18 PM, Andres Freund <[email protected]>
> wrote:
>> On 2015-03-11 06:54:16 +0000, Fujii Masao wrote:
>>> Add GUC to enable compression of full page images stored in WAL.
>>
>> This triggers a couple warnings here (gcc 4.9 and 5):
>>
>> /home/andres/src/postgresql/src/backend/access/transam/xloginsert.c: In
>> function 'XLogInsert':
>> /home/andres/src/postgresql/src/backend/access/transam/xloginsert.c:670:5:
>> warning: 'cbimg.hole_length' may be used uninitialized in this function
>> [-Wmaybe-uninitialized]
>> memcpy(scratch, &cbimg,
>> ^
>> /home/andres/src/postgresql/src/backend/access/transam/xloginsert.c:494:33:
>> note: 'cbimg.hole_length' was declared here
>> XLogRecordBlockCompressHeader cbimg;
>> ^
>> /home/andres/src/postgresql/src/backend/access/transam/xloginsert.c:668:20:
>> warning: 'hole_length' may be used uninitialized in this function
>> [-Wmaybe-uninitialized]
>> if (hole_length != 0 && is_compressed)
>> ^
>> /home/andres/src/postgresql/src/backend/access/transam/xloginsert.c:497:10:
>> note: 'hole_length' was declared here
>> uint16 hole_length;
>>
>> I've not checked whether they're spurious or not.
Thanks for the report! We can suppress those warnings just by
initializing cbimg.hole_length and hole_length. But I'd like to apply
the attached patch which not only initializes the variable but also
refactors the related code.
Regards,
--
Fujii Masao
*** a/src/backend/access/transam/xloginsert.c
--- b/src/backend/access/transam/xloginsert.c
***************
*** 491,501 **** XLogRecordAssemble(RmgrId rmid, uint8 info,
bool needs_data;
XLogRecordBlockHeader bkpb;
XLogRecordBlockImageHeader bimg;
! XLogRecordBlockCompressHeader cbimg;
bool samerel;
bool is_compressed = false;
- uint16 hole_length;
- uint16 hole_offset;
if (!regbuf->in_use)
continue;
--- 491,499 ----
bool needs_data;
XLogRecordBlockHeader bkpb;
XLogRecordBlockImageHeader bimg;
! XLogRecordBlockCompressHeader cbimg = {0};
bool samerel;
bool is_compressed = false;
if (!regbuf->in_use)
continue;
***************
*** 558,578 **** XLogRecordAssemble(RmgrId rmid, uint8 info,
upper > lower &&
upper <= BLCKSZ)
{
! hole_offset = lower;
! hole_length = upper - lower;
}
else
{
/* No "hole" to compress out */
! hole_offset = 0;
! hole_length = 0;
}
}
else
{
/* Not a standard page header, don't try to eliminate "hole" */
! hole_offset = 0;
! hole_length = 0;
}
/*
--- 556,576 ----
upper > lower &&
upper <= BLCKSZ)
{
! bimg.hole_offset = lower;
! cbimg.hole_length = upper - lower;
}
else
{
/* No "hole" to compress out */
! bimg.hole_offset = 0;
! cbimg.hole_length = 0;
}
}
else
{
/* Not a standard page header, don't try to eliminate "hole" */
! bimg.hole_offset = 0;
! cbimg.hole_length = 0;
}
/*
***************
*** 581,587 **** XLogRecordAssemble(RmgrId rmid, uint8 info,
if (wal_compression)
{
is_compressed =
! XLogCompressBackupBlock(page, hole_offset, hole_length,
regbuf->compressed_page,
&compressed_len);
}
--- 579,586 ----
if (wal_compression)
{
is_compressed =
! XLogCompressBackupBlock(page, bimg.hole_offset,
! cbimg.hole_length,
regbuf->compressed_page,
&compressed_len);
}
***************
*** 595,619 **** XLogRecordAssemble(RmgrId rmid, uint8 info,
rdt_datas_last->next = ®buf->bkp_rdatas[0];
rdt_datas_last = rdt_datas_last->next;
! bimg.bimg_info = (hole_length == 0) ? 0 : BKPIMAGE_HAS_HOLE;
if (is_compressed)
{
bimg.length = compressed_len;
- bimg.hole_offset = hole_offset;
bimg.bimg_info |= BKPIMAGE_IS_COMPRESSED;
- if (hole_length != 0)
- cbimg.hole_length = hole_length;
rdt_datas_last->data = regbuf->compressed_page;
rdt_datas_last->len = compressed_len;
}
else
{
! bimg.length = BLCKSZ - hole_length;
! bimg.hole_offset = hole_offset;
! if (hole_length == 0)
{
rdt_datas_last->data = page;
rdt_datas_last->len = BLCKSZ;
--- 594,614 ----
rdt_datas_last->next = ®buf->bkp_rdatas[0];
rdt_datas_last = rdt_datas_last->next;
! bimg.bimg_info = (cbimg.hole_length == 0) ? 0 : BKPIMAGE_HAS_HOLE;
if (is_compressed)
{
bimg.length = compressed_len;
bimg.bimg_info |= BKPIMAGE_IS_COMPRESSED;
rdt_datas_last->data = regbuf->compressed_page;
rdt_datas_last->len = compressed_len;
}
else
{
! bimg.length = BLCKSZ - cbimg.hole_length;
! if (cbimg.hole_length == 0)
{
rdt_datas_last->data = page;
rdt_datas_last->len = BLCKSZ;
***************
*** 622,634 **** XLogRecordAssemble(RmgrId rmid, uint8 info,
{
/* must skip the hole */
rdt_datas_last->data = page;
! rdt_datas_last->len = hole_offset;
rdt_datas_last->next = ®buf->bkp_rdatas[1];
rdt_datas_last = rdt_datas_last->next;
! rdt_datas_last->data = page + (hole_offset + hole_length);
! rdt_datas_last->len = BLCKSZ - (hole_offset + hole_length);
}
}
--- 617,631 ----
{
/* must skip the hole */
rdt_datas_last->data = page;
! rdt_datas_last->len = bimg.hole_offset;
rdt_datas_last->next = ®buf->bkp_rdatas[1];
rdt_datas_last = rdt_datas_last->next;
! rdt_datas_last->data =
! page + (bimg.hole_offset + cbimg.hole_length);
! rdt_datas_last->len =
! BLCKSZ - (bimg.hole_offset + cbimg.hole_length);
}
}
***************
*** 665,671 **** XLogRecordAssemble(RmgrId rmid, uint8 info,
{
memcpy(scratch, &bimg, SizeOfXLogRecordBlockImageHeader);
scratch += SizeOfXLogRecordBlockImageHeader;
! if (hole_length != 0 && is_compressed)
{
memcpy(scratch, &cbimg,
SizeOfXLogRecordBlockCompressHeader);
--- 662,668 ----
{
memcpy(scratch, &bimg, SizeOfXLogRecordBlockImageHeader);
scratch += SizeOfXLogRecordBlockImageHeader;
! if (cbimg.hole_length != 0 && is_compressed)
{
memcpy(scratch, &cbimg,
SizeOfXLogRecordBlockCompressHeader);
--
Sent via pgsql-committers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
