MIME-Version: 1.0
Content-Type: multipart/mixed;
 boundary="=_b7eb9c10db995b53aecf71d59d7ae55d"
Date: Thu, 16 Nov 2017 15:05:59 +0300
From: a.akenteva@postgrespro.ru
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers@postgresql.org, pgsql-hackers-owner@postgresql.org
Subject: Re: [HACKERS] A weird bit in pg_upgrade/exec.c
In-Reply-To: <21596.1510244246@sss.pgh.pa.us>
References: <f9266a85d918a3cf3a386b5148aee666@postgrespro.ru>
 <21596.1510244246@sss.pgh.pa.us>
Message-ID: <ba81b93e77c4bc1778ae6f0def7be883@postgrespro.ru>
User-Agent: Roundcube Webmail/1.2.4
Precedence: bulk

--=_b7eb9c10db995b53aecf71d59d7ae55d
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset=US-ASCII;
 format=flowed

Tom Lane <tgl@sss.pgh.pa.us> writes:
> Yeah, the way it is now seems outright broken.  It will try to do
> get_bin_version on the new cluster before having done validate_exec
> there, violating its own comment.
> 
> I think we should change this as a bug fix, independently of whatever
> else you had in mind to do here.

I see this bug is now fixed in pg_upgrade/exec.c => check_bin_dir().

There's another bit exactly like that in check_data_dir() though.
We use global variables instead of using the argument passed to the 
function,
which makes the function not reusable. Sorry if I mentioned it too 
vaguely
in the previous letter.

I attached a patch with the change that would fix it.
In pg_upgrade/exec.c => check_data_dir() I substituted these two lines
     old_cluster.major_version = get_major_server_version(&old_cluster);
     new_cluster.major_version = get_major_server_version(&new_cluster);
with this line:
     cluster->major_version = get_major_server_version(cluster);
and changed the comment accordingly.
--=_b7eb9c10db995b53aecf71d59d7ae55d
Content-Transfer-Encoding: base64
Content-Type: text/x-diff;
 name=Fixed-exec-c.patch
Content-Disposition: attachment;
 filename=Fixed-exec-c.patch;
 size=1047

RnJvbSA5YjYzYTdmYzVkMWRjZTZkMjU0YWVjMGFiZTQxMmYyYzFjZDVmODg5IE1vbiBTZXAgMTcg
MDA6MDA6MDAgMjAwMQpGcm9tOiBBa2VudGV2YSBBbm5hIDxhLmFrZW50ZXZhQHBvc3RncmVzcHJv
LnJ1PgpEYXRlOiBUaHUsIDE2IE5vdiAyMDE3IDEzOjI1OjI5ICswMzAwClN1YmplY3Q6IFtQQVRD
SF0gRml4ZWQgY2hlY2tfZGF0YV9kaXIgaW4gcGdfdXBncmFkZS9leGVjLmMKCkNoYW5nZWQgdXNp
bmcgZ2xvYmFsIHZhcmlhYmxlcyAob2xkX2NsdXN0ZXIgYW5kIG5ld19jbHVzdGVyKSBpbnRvCnVz
aW5nIHRoZSBhcmd1bWVudCBvZiB0aGUgY2hlY2tfZGF0YV9kaXIgZnVuY3Rpb24uCi0tLQogc3Jj
L2Jpbi9wZ191cGdyYWRlL2V4ZWMuYyB8IDUgKystLS0KIDEgZmlsZSBjaGFuZ2VkLCAyIGluc2Vy
dGlvbnMoKyksIDMgZGVsZXRpb25zKC0pCgpkaWZmIC0tZ2l0IGEvc3JjL2Jpbi9wZ191cGdyYWRl
L2V4ZWMuYyBiL3NyYy9iaW4vcGdfdXBncmFkZS9leGVjLmMKaW5kZXggODEwYTVhMC4uZjVjZDc0
ZiAxMDA2NDQKLS0tIGEvc3JjL2Jpbi9wZ191cGdyYWRlL2V4ZWMuYworKysgYi9zcmMvYmluL3Bn
X3VwZ3JhZGUvZXhlYy5jCkBAIC0zMzEsOSArMzMxLDggQEAgY2hlY2tfZGF0YV9kaXIoQ2x1c3Rl
ckluZm8gKmNsdXN0ZXIpCiB7CiAJY29uc3QgY2hhciAqcGdfZGF0YSA9IGNsdXN0ZXItPnBnZGF0
YTsKIAotCS8qIGdldCBvbGQgYW5kIG5ldyBjbHVzdGVyIHZlcnNpb25zICovCi0Jb2xkX2NsdXN0
ZXIubWFqb3JfdmVyc2lvbiA9IGdldF9tYWpvcl9zZXJ2ZXJfdmVyc2lvbigmb2xkX2NsdXN0ZXIp
OwotCW5ld19jbHVzdGVyLm1ham9yX3ZlcnNpb24gPSBnZXRfbWFqb3Jfc2VydmVyX3ZlcnNpb24o
Jm5ld19jbHVzdGVyKTsKKwkvKiBnZXQgdGhlIGNsdXN0ZXIgdmVyc2lvbiAqLworCWNsdXN0ZXIt
Pm1ham9yX3ZlcnNpb24gPSBnZXRfbWFqb3Jfc2VydmVyX3ZlcnNpb24oY2x1c3Rlcik7CiAKIAlj
aGVja19zaW5nbGVfZGlyKHBnX2RhdGEsICIiKTsKIAljaGVja19zaW5nbGVfZGlyKHBnX2RhdGEs
ICJiYXNlIik7Ci0tIAoyLjcuNAoK
--=_b7eb9c10db995b53aecf71d59d7ae55d--