timeline garbage in pg_basebackup (was [HACKERS] gcc 4.6 warnings -Wunused-but-set-variable)
On tis, 2011-03-29 at 23:48 +0300, Peter Eisentraut wrote: The line I marked in pg_basebackup.c might be an actual problem: It goes through a whole lot to figure out the timeline and then doesn't do anything with it. This hasn't been addressed yet. It doesn't manifest itself as an actual problem, but it looks as though someone had intended something in that code and the code doesn't do that. -- 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] gcc 4.6 warnings -Wunused-but-set-variable
On tis, 2011-03-29 at 23:48 +0300, Peter Eisentraut wrote: As you might have heard, GCC 4.6 was released the other day. It generates a bunch of new warnings with the PostgreSQL source code, most of which belong to the new warning scenario -Wunused-but-set-variable, which is included in -Wall. In case someone else tries that, I have filed a bug with GCC regarding some of the other warnings: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48778 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: timeline garbage in pg_basebackup (was [HACKERS] gcc 4.6 warnings -Wunused-but-set-variable)
On Wed, Apr 27, 2011 at 18:55, Peter Eisentraut pete...@gmx.net wrote: On tis, 2011-03-29 at 23:48 +0300, Peter Eisentraut wrote: The line I marked in pg_basebackup.c might be an actual problem: It goes through a whole lot to figure out the timeline and then doesn't do anything with it. This hasn't been addressed yet. It doesn't manifest itself as an actual problem, but it looks as though someone had intended something in that code and the code doesn't do that. Do you have a ref to the actual problem? The subject change killed my threading, the email was trimmed to not include the actual problem, and it appears not to be listed on the open items list... ;) -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: timeline garbage in pg_basebackup (was [HACKERS] gcc 4.6 warnings -Wunused-but-set-variable)
On ons, 2011-04-27 at 19:17 +0200, Magnus Hagander wrote: On Wed, Apr 27, 2011 at 18:55, Peter Eisentraut pete...@gmx.net wrote: On tis, 2011-03-29 at 23:48 +0300, Peter Eisentraut wrote: The line I marked in pg_basebackup.c might be an actual problem: It goes through a whole lot to figure out the timeline and then doesn't do anything with it. This hasn't been addressed yet. It doesn't manifest itself as an actual problem, but it looks as though someone had intended something in that code and the code doesn't do that. Do you have a ref to the actual problem? The subject change killed my threading, the email was trimmed to not include the actual problem, and it appears not to be listed on the open items list... ;) In BaseBackup(), the variable timeline is assigned in a somewhat elaborate fashion, but then the result is not used for anything. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: timeline garbage in pg_basebackup (was [HACKERS] gcc 4.6 warnings -Wunused-but-set-variable)
On Wed, Apr 27, 2011 at 20:21, Peter Eisentraut pete...@gmx.net wrote: On ons, 2011-04-27 at 19:17 +0200, Magnus Hagander wrote: On Wed, Apr 27, 2011 at 18:55, Peter Eisentraut pete...@gmx.net wrote: On tis, 2011-03-29 at 23:48 +0300, Peter Eisentraut wrote: The line I marked in pg_basebackup.c might be an actual problem: It goes through a whole lot to figure out the timeline and then doesn't do anything with it. This hasn't been addressed yet. It doesn't manifest itself as an actual problem, but it looks as though someone had intended something in that code and the code doesn't do that. Do you have a ref to the actual problem? The subject change killed my threading, the email was trimmed to not include the actual problem, and it appears not to be listed on the open items list... ;) In BaseBackup(), the variable timeline is assigned in a somewhat elaborate fashion, but then the result is not used for anything. Ah, I see it. What happened there is I accidentally included it when I split my patches apart. It's required in the stream WAL in parallel to the base backup to decrease requirements on wal_keep_segmtents. But that patch was postponed since there were still bugs in it, and it wasn't entirely feature-complete, and we were pretty far past feature-freeze. So it's not needed in 9.1. I'll rip it out and move it over to the patch once it's ready to go for 9.2. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- 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] gcc 4.6 warnings -Wunused-but-set-variable
On Tue, Mar 29, 2011 at 4:48 PM, Peter Eisentraut pete...@gmx.net wrote: As you might have heard, GCC 4.6 was released the other day. It generates a bunch of new warnings with the PostgreSQL source code, most of which belong to the new warning scenario -Wunused-but-set-variable, which is included in -Wall. Attached is a patch that gets rid of most of these. As you can see, most of these remove real leftover garbage. The line I marked in pg_basebackup.c might be an actual problem: It goes through a whole lot to figure out the timeline and then doesn't do anything with it. In some other cases, however, one might argue that the changes lose some clarity, such as when dropping the return value of strtoul() or va_arg(). How should we proceed? In any case, my patch should be re-reviewed for any possible side effects that I might have hastily removed. In the case of variable.c, it is entirely unclear that there's any point in calling strtoul() at all. Maybe we should just remove that and the following Assert() as well. In parse_utilcmd.c, do we need to look up the collation OID if we're just discarding it anyway? In the case of the va_arg() calls, maybe something like /* advance arg position, but ignore result */? -- 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
[HACKERS] gcc 4.6 warnings -Wunused-but-set-variable
As you might have heard, GCC 4.6 was released the other day. It
generates a bunch of new warnings with the PostgreSQL source code, most
of which belong to the new warning scenario -Wunused-but-set-variable,
which is included in -Wall.
Attached is a patch that gets rid of most of these. As you can see,
most of these remove real leftover garbage. The line I marked in
pg_basebackup.c might be an actual problem: It goes through a whole lot
to figure out the timeline and then doesn't do anything with it. In
some other cases, however, one might argue that the changes lose some
clarity, such as when dropping the return value of strtoul() or
va_arg(). How should we proceed? In any case, my patch should be
re-reviewed for any possible side effects that I might have hastily
removed.
diff --git i/contrib/isn/isn.c w/contrib/isn/isn.c
index 46e904b..b698cb0 100644
--- i/contrib/isn/isn.c
+++ w/contrib/isn/isn.c
@@ -341,8 +341,7 @@ ean2isn(ean13 ean, bool errorOK, ean13 *result, enum isn_type accept)
enum isn_type type = INVALID;
char buf[MAXEAN13LEN + 1];
- char *firstdig,
- *aux;
+ char *aux;
unsigned digval;
unsigned search;
ean13 ret = ean;
@@ -354,7 +353,7 @@ ean2isn(ean13 ean, bool errorOK, ean13 *result, enum isn_type accept)
/* convert the number */
search = 0;
- firstdig = aux = buf + 13;
+ aux = buf + 13;
*aux = '\0';/* terminate string; aux points to last digit */
do
{
@@ -528,8 +527,7 @@ ean2string(ean13 ean, bool errorOK, char *result, bool shortType)
const unsigned (*TABLE_index)[2];
enum isn_type type = INVALID;
- char *firstdig,
- *aux;
+ char *aux;
unsigned digval;
unsigned search;
char valid = '\0'; /* was the number initially written with a
@@ -546,7 +544,7 @@ ean2string(ean13 ean, bool errorOK, char *result, bool shortType)
/* convert the number */
search = 0;
- firstdig = aux = result + MAXEAN13LEN;
+ aux = result + MAXEAN13LEN;
*aux = '\0';/* terminate string; aux points to last digit */
*--aux = valid;/* append '!' for numbers with invalid but
* corrected check digit */
diff --git i/contrib/pageinspect/fsmfuncs.c w/contrib/pageinspect/fsmfuncs.c
index eca3230..38c4e23 100644
--- i/contrib/pageinspect/fsmfuncs.c
+++ w/contrib/pageinspect/fsmfuncs.c
@@ -35,7 +35,6 @@ Datum
fsm_page_contents(PG_FUNCTION_ARGS)
{
bytea *raw_page = PG_GETARG_BYTEA_P(0);
- int raw_page_size;
StringInfoData sinfo;
FSMPage fsmpage;
int i;
@@ -45,7 +44,6 @@ fsm_page_contents(PG_FUNCTION_ARGS)
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
(errmsg(must be superuser to use raw page functions;
- raw_page_size = VARSIZE(raw_page) - VARHDRSZ;
fsmpage = (FSMPage) PageGetContents(VARDATA(raw_page));
initStringInfo(sinfo);
diff --git i/contrib/pgcrypto/pgp-s2k.c w/contrib/pgcrypto/pgp-s2k.c
index ef16caf..349234e 100644
--- i/contrib/pgcrypto/pgp-s2k.c
+++ w/contrib/pgcrypto/pgp-s2k.c
@@ -39,14 +39,12 @@ static int
calc_s2k_simple(PGP_S2K *s2k, PX_MD *md, const uint8 *key,
unsigned key_len)
{
- unsigned md_bs,
-md_rlen;
+ unsigned md_rlen;
uint8 buf[PGP_MAX_DIGEST];
unsigned preload;
unsigned remain;
uint8 *dst = s2k-key;
- md_bs = px_md_block_size(md);
md_rlen = px_md_result_size(md);
remain = s2k-key_len;
@@ -83,14 +81,12 @@ calc_s2k_simple(PGP_S2K *s2k, PX_MD *md, const uint8 *key,
static int
calc_s2k_salted(PGP_S2K *s2k, PX_MD *md, const uint8 *key, unsigned key_len)
{
- unsigned md_bs,
-md_rlen;
+ unsigned md_rlen;
uint8 buf[PGP_MAX_DIGEST];
unsigned preload = 0;
uint8 *dst;
unsigned remain;
- md_bs = px_md_block_size(md);
md_rlen = px_md_result_size(md);
dst = s2k-key;
@@ -129,8 +125,7 @@ static int
calc_s2k_iter_salted(PGP_S2K *s2k, PX_MD *md, const uint8 *key,
unsigned key_len)
{
- unsigned md_bs,
-md_rlen;
+ unsigned md_rlen;
uint8 buf[PGP_MAX_DIGEST];
uint8 *dst;
unsigned preload = 0;
@@ -143,7 +138,6 @@ calc_s2k_iter_salted(PGP_S2K *s2k, PX_MD *md, const uint8 *key,
cval = s2k-iter;
count = ((unsigned) 16 + (cval 15)) ((cval 4) + 6);
- md_bs = px_md_block_size(md);
md_rlen = px_md_result_size(md);
remain = s2k-key_len;
diff --git i/contrib/pgcrypto/px-hmac.c w/contrib/pgcrypto/px-hmac.c
index 16abc43..36efabd 100644
--- i/contrib/pgcrypto/px-hmac.c
+++ w/contrib/pgcrypto/px-hmac.c
@@ -52,13 +52,11 @@ static void
hmac_init(PX_HMAC *h, const uint8 *key, unsigned klen)
{
unsigned bs,
-hlen,
i;
uint8 *keybuf;
PX_MD *md = h-md;
bs = px_md_block_size(md);
- hlen = px_md_result_size(md);
keybuf = px_alloc(bs);
memset(keybuf, 0, bs);
diff --git i/contrib/pgcrypto/px.c w/contrib/pgcrypto/px.c
index 768c7c3..e3f5e26 100644
--- i/contrib/pgcrypto/px.c
+++ w/contrib/pgcrypto/px.c
@@ -162,14 +162,12 @@ combo_init(PX_Combo *cx, const uint8 *key, unsigned klen,
const uint8 *iv, unsigned ivlen)
{
int err;
- unsigned bs,
-ks,
+ unsigned ks,
ivs;
