From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gnu.wildebeest.org (wildebeest.demon.nl [212.238.236.112]) by sourceware.org (Postfix) with ESMTPS id 60D01384A015 for ; Wed, 28 Jul 2021 14:55:06 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 60D01384A015 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=klomp.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=klomp.org Received: from tarox.wildebeest.org (83-87-18-245.cable.dynamic.v4.ziggo.nl [83.87.18.245]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by gnu.wildebeest.org (Postfix) with ESMTPSA id AB1C830291A9; Wed, 28 Jul 2021 16:55:01 +0200 (CEST) Received: by tarox.wildebeest.org (Postfix, from userid 1000) id C54E844C4264; Wed, 28 Jul 2021 16:55:00 +0200 (CEST) Message-ID: <308d7b3305efae40152abbf3b4c9c06b3d6a22fa.camel@klomp.org> Subject: Re: [Bug debuginfod/27983] ignore duplicate urls From: Mark Wielaard To: Noah Sanci , elfutils-devel@sourceware.org Date: Wed, 28 Jul 2021 16:55:00 +0200 In-Reply-To: References: <7b021d11929e451d77961ab183cd97f3329a6dce.camel@klomp.org> Content-Type: multipart/mixed; boundary="=-Rl8QPUtxtDSw2e0Avhey" X-Mailer: Evolution 3.28.5 (3.28.5-10.el7) Mime-Version: 1.0 X-Spam-Status: No, score=-9.3 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_BADIPHTTP, KAM_DMARC_STATUS, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: elfutils-devel@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Elfutils-devel mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 28 Jul 2021 14:55:10 -0000 --=-Rl8QPUtxtDSw2e0Avhey Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi Noah, On Mon, 2021-07-26 at 12:29 -0400, Noah Sanci via Elfutils-devel wrote: > The realloc returning NULL issue has been resolved and the patch > successfully rebased onto master. Please find these improvements > attached. This looks really good, but I found some small issues. First it turns out reallocarray isn't available on some older systems. This is easy to workaround though since it is a fairly simple function we could provide ourselves if it isn't there. The attached patch does that. I'll push it if it looks good. Second there is a small memory leak found by valgrind. We only clean up the server_url_list on failure, but we should also clean it up when we are done on success: diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod- client.c index 9d049d33..0f65f115 100644 --- a/debuginfod/debuginfod-client.c +++ b/debuginfod/debuginfod-client.c @@ -1191,6 +1191,9 @@ debuginfod_query_server (debuginfod_client *c, } =20 free (data); + for (int i =3D 0; i < num_urls; ++i) + free(server_url_list[i]); + free(server_url_list); free (server_urls); =20 /* don't close fd - we're returning it */ Finally on some systems, but not all, one of the tests in run- debuginfod-find.sh failed. In particular this one: # check out the debuginfod logs for the new style status lines # cat vlog$PORT2 grep -q 'UA:.*XFF:.*GET /buildid/.* 200 ' vlog$PORT2 grep -q 'UA:.*XFF:.*GET /metrics 200 ' vlog$PORT2 grep -q 'UA:.*XFF:.*GET /badapi 503 ' vlog$PORT2 grep -q 'UA:.*XFF:.*GET /buildid/deadbeef.* 404 ' vlog$PORT2 That last one changed error message from 404 to 503. This seems related to the setting of DEBUGINFOD_URLS earlier by the new test you added. If I change that to: diff --git a/tests/run-debuginfod-find.sh b/tests/run-debuginfod-find.sh index c2c3b9c3..ec639a38 100755 --- a/tests/run-debuginfod-find.sh +++ b/tests/run-debuginfod-find.sh @@ -367,8 +367,7 @@ wait_ready $PORT1 'found_sourcerefs_total{source=3D".rp= m archive"}' $sourcefiles ######################################################################## # PR27983 ensure no duplicate urls are used in when querying servers for f= iles rm -rf $DEBUGINFOD_CACHE_PATH # clean it from previous tests -export DEBUGINFOD_URLS=3D"http://127.0.0.1:$PORT1 http://127.0.0.1:$PORT1 = http://127.0.0.1:$PORT1 http:127.0.0.1:7999" -env LD_LIBRARY_PATH=3D$ldpath ${abs_top_builddir}/debuginfod/debuginfod-fi= nd -v executable $BUILDID2 > vlog4 2>&1 || true +env DEBUGINFOD_URLS=3D"http://127.0.0.1:$PORT1 http://127.0.0.1:$PORT1 htt= p://127.0.0.1:$PORT1 http:127.0.0.1:7999" LD_LIBRARY_PATH=3D$ldpath ${abs_t= op_builddir}/debuginfod/debuginfod-find -v executable $BUILDID2 > vlog4 2>&= 1 || true tempfiles vlog4 if [ $( grep -c 'duplicate url: http://127.0.0.1:'$PORT1'.*' vlog4 ) -ne 2= ]; then echo "Duplicated servers remain"; Everything seems to pass everywhere. run-debuginfod-find.sh is really convoluted and we really shouldn't add more and more interacting tests to it. But if we do maybe we should use the env DEBUGINFOD_URLS=3D... trick to minimize changing other tests in the same file. If you agree with the above two changes, could you resubmit the patch with those? Thanks, Mark --=-Rl8QPUtxtDSw2e0Avhey Content-Disposition: inline; filename*0=0001-lib-Add-static-inline-reallocarray-fallback-function.pat; filename*1=ch Content-Type: text/x-patch; name="0001-lib-Add-static-inline-reallocarray-fallback-function.patch"; charset="UTF-8" Content-Transfer-Encoding: base64 RnJvbSA2MTIxMGY5MWFmYTQwODRhNDZiZDBmODRkMTJhYTZkM2YyOWMxMjcxIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBNYXJrIFdpZWxhYXJkIDxtYXJrQGtsb21wLm9yZz4KRGF0ZTog V2VkLCAyOCBKdWwgMjAyMSAxNjo0NjozNiArMDIwMApTdWJqZWN0OiBbUEFUQ0hdIGxpYjogQWRk IHN0YXRpYyBpbmxpbmUgcmVhbGxvY2FycmF5IGZhbGxiYWNrIGZ1bmN0aW9uCgpTaWduZWQtb2Zm LWJ5OiBNYXJrIFdpZWxhYXJkIDxtYXJrQGtsb21wLm9yZz4KLS0tCiBDaGFuZ2VMb2cgICAgIHwg IDQgKysrKwogY29uZmlndXJlLmFjICB8ICAzICsrKwogbGliL0NoYW5nZUxvZyB8ICA0ICsrKysK IGxpYi9zeXN0ZW0uaCAgfCAxNCArKysrKysrKysrKysrKwogNCBmaWxlcyBjaGFuZ2VkLCAyNSBp bnNlcnRpb25zKCspCgpkaWZmIC0tZ2l0IGEvQ2hhbmdlTG9nIGIvQ2hhbmdlTG9nCmluZGV4IGEw MWY2ZjlmLi4xMmI4ZjQwMyAxMDA2NDQKLS0tIGEvQ2hhbmdlTG9nCisrKyBiL0NoYW5nZUxvZwpA QCAtMSwzICsxLDcgQEAKKzIwMjEtMDctMjggIE1hcmsgV2llbGFhcmQgIDxtYXJrQGtsb21wLm9y Zz4KKworCSogY29uZmlndXJlLmFjIChBQ19DSEVDS19ERUNMUyk6IEFkZCByZWFsbG9jYXJyYXkg Y2hlY2suCisKIDIwMjEtMDUtMjIgIE1hcmsgV2llbGFhcmQgIDxtYXJrQGtsb21wLm9yZz4KIAog CSogY29uZmlndXJlLmFjIChBQ19JTklUKTogU2V0IHZlcnNpb24gdG8gMC4xODUuCmRpZmYgLS1n aXQgYS9jb25maWd1cmUuYWMgYi9jb25maWd1cmUuYWMKaW5kZXggYjM0OGE3MTcuLjdjYWZmMmM1 IDEwMDY0NAotLS0gYS9jb25maWd1cmUuYWMKKysrIGIvY29uZmlndXJlLmFjCkBAIC00MjUsNiAr NDI1LDkgQEAgQUNfQ0hFQ0tfREVDTFMoW3Bvd2Vyb2YyXSxbXSxbXSxbI2luY2x1ZGUgPHN5cy9w YXJhbS5oPl0pCiBBQ19DSEVDS19ERUNMUyhbbWVtcGNweV0sW10sW10sCiAgICAgICAgICAgICAg ICBbI2RlZmluZSBfR05VX1NPVVJDRQogICAgICAgICAgICAgICAgICNpbmNsdWRlIDxzdHJpbmcu aD5dKQorQUNfQ0hFQ0tfREVDTFMoW3JlYWxsb2NhcnJheV0sW10sW10sCisgICAgICAgICAgICAg ICBbI2RlZmluZSBfR05VX1NPVVJDRQorICAgICAgICAgICAgICAgICNpbmNsdWRlIDxzdGRsaWIu aD5dKQogCiBBQ19DSEVDS19GVU5DUyhbcHJvY2Vzc192bV9yZWFkdl0pCiAKZGlmZiAtLWdpdCBh L2xpYi9DaGFuZ2VMb2cgYi9saWIvQ2hhbmdlTG9nCmluZGV4IGRkM2ViY2FiLi40NDM2NmZlYyAx MDA2NDQKLS0tIGEvbGliL0NoYW5nZUxvZworKysgYi9saWIvQ2hhbmdlTG9nCkBAIC0xLDMgKzEs NyBAQAorMjAyMS0wNy0yOCAgTWFyayBXaWVsYWFyZCAgPG1hcmtAa2xvbXAub3JnPgorCisJKiBz eXN0ZW0uaCAocmVhbGxvY2FycmF5KTogTmV3IHN0YXRpYyBpbmxpbmUgZmFsbGJhY2sgZnVuY3Rp b24uCisKIDIwMjEtMDQtMTkgIE1hcnRpbiBMaXNrYSAgPG1saXNrYUBzdXNlLmN6PgogCiAJKiBz eXN0ZW0uaCAoc3RhcnRzd2l0aCk6IE5ldyBmdW5jdGlvbi4KZGlmZiAtLWdpdCBhL2xpYi9zeXN0 ZW0uaCBiL2xpYi9zeXN0ZW0uaAppbmRleCBjZGYxOGVkNy4uNThkOWRlZWUgMTAwNjQ0Ci0tLSBh L2xpYi9zeXN0ZW0uaAorKysgYi9saWIvc3lzdGVtLmgKQEAgLTM4LDYgKzM4LDcgQEAKICNpbmNs dWRlIDxieXRlc3dhcC5oPgogI2luY2x1ZGUgPHVuaXN0ZC5oPgogI2luY2x1ZGUgPHN0cmluZy5o PgorI2luY2x1ZGUgPHN0ZGxpYi5oPgogCiAjaWYgX19CWVRFX09SREVSID09IF9fTElUVExFX0VO RElBTgogIyBkZWZpbmUgTEUzMihuKQkobikKQEAgLTcwLDYgKzcxLDE5IEBACiAgICAgKCh2b2lk ICopICgoY2hhciAqKSBtZW1jcHkgKGRlc3QsIHNyYywgbikgKyAoc2l6ZV90KSBuKSkKICNlbmRp ZgogCisjaWYgIUhBVkVfREVDTF9SRUFMTE9DQVJSQVkKK3N0YXRpYyBpbmxpbmUgdm9pZCAqCity ZWFsbG9jYXJyYXkgKHZvaWQgKnB0ciwgc2l6ZV90IG5tZW1iLCBzaXplX3Qgc2l6ZSkKK3sKKyAg aWYgKHNpemUgPiAwICYmIG5tZW1iID4gU0laRV9NQVggLyBzaXplKQorICAgIHsKKyAgICAgIGVy cm5vID0gRU5PTUVNOworICAgICAgcmV0dXJuIE5VTEw7CisgICAgfQorICByZXR1cm4gcmVhbGxv YyAocHRyLCBubWVtYiAqIHNpemUpOworfQorI2VuZGlmCisKIC8qIFJldHVybiBUUlVFIGlmIHRo ZSBzdGFydCBvZiBTVFIgbWF0Y2hlcyBQUkVGSVgsIEZBTFNFIG90aGVyd2lzZS4gICovCiAKIHN0 YXRpYyBpbmxpbmUgaW50Ci0tIAoyLjE4LjQKCg== --=-Rl8QPUtxtDSw2e0Avhey--