From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id 2790D3858004 for ; Sun, 28 Mar 2021 17:52:10 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 2790D3858004 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id 12SHp2j9005285 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Sun, 28 Mar 2021 13:51:07 -0400 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 12SHp2j9005285 Received: from [10.0.0.11] (192-222-157-6.qc.cable.ebox.net [192.222.157.6]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 497EE1E783; Sun, 28 Mar 2021 13:51:02 -0400 (EDT) Subject: Re: [PATCH v3] [PR gdb/27614] gdb-add-index fails on symlinks. To: Lancelot SIX Cc: gdb-patches@sourceware.org References: <20210327172758.30356-1-lsix@lancelotsix.com> <981b48ee-133a-7f06-9e5d-d1c331f360d1@polymtl.ca> From: Simon Marchi Message-ID: <2cba4c43-f085-ace3-d3f2-5f30bff426f3@polymtl.ca> Date: Sun, 28 Mar 2021 13:51:02 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Sun, 28 Mar 2021 17:51:02 +0000 X-Spam-Status: No, score=-4.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, NICE_REPLY_A, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_PASS, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 28 Mar 2021 17:52:11 -0000 On 2021-03-28 12:56 p.m., Lancelot SIX wrote: > Le Sun, Mar 28, 2021 at 12:11:22PM -0400, Simon Marchi a écrit : >> >> >> On 2021-03-27 1:27 p.m., Lancelot SIX via Gdb-patches wrote: >>> Since V2: >>> - Use GDB to follow symlink instead of readlink. Unlike >>> readlink, GDB is guaranteed to be available. >>> >>> Since V1: >>> - Replace '&>/dev/null' with '>/dev/null 2>&1' >>> >>> -- >>> >>> PR 27614 shows that gdb-add-index fails to generate the index when its >>> argument is a symlink. >>> >>> The following one liner illustrates the reported problem: >>> >>> $ echo 'int main(){}'|gcc -g -x c -;ln -s a.out symlink;gdb-add-index symlink >>> gdb-add-index: No index was created for symlink >>> gdb-add-index: [Was there no debuginfo? Was there already an index?] >>> $ ls -l >>> -rwxr-xr-x 1 25712 Mar 19 23:05 a.out* >>> -rw------- 1 8277 Mar 19 23:05 a.out.gdb-index >>> lrwxrwxrwx 1 5 Mar 19 23:05 symlink -> a.out* >>> >>> GDB generates the .gdb-index file with a name that matches the name of >>> the actual program (a.out.gdb-index here), not the symlink that >>> references it. The remaining of the script is looking for a file named >>> after the provided argument (would be 'symlink.gdb-index' in our >>> example). >>> >>> The common option to solve such issue would be to use readlink to follow >>> the symlink. Unfortunately, this command is not available in the POSIX >>> standard. This commit therefore proposes to use GDB itself to identify >>> where the symlink points to. This requires some parsing of GDB output. >>> The added test should be enough to detect regression if GDB where to >>> change the way it formats its output. >> >> I preferred your previous approach, compared to relying on a maintenance >> command. Relying on a maintenance command is fine in tests, for example, >> but here somebody could use gdb-add-index from a given GDB version with >> a GDB of a different version. >> >> GDB=/my/newer/gdb gdb-add-index a.out > > Yes, agreed. I was reluctant to rely on maint command in the first > place, I should have kept it that way! > >> >> In the previous review, you said: >> >>>> Would it work with just `readlink `? >> >>> This would fail if $file is a symlink to a symlink. This is what >>> ldconfig usually does (libfoo.so -> libfoo.so.x -> libfoo.so.x.y). >> >> Can't you just call it in a loop then? >> >> while file is a symlink: >> file=readlink $file >> > > Yes. It would look somethink like: > > > if test -L "$1"; then > if command -v readlink >/dev/null 2>&1; then > file="$1" > while test -L "$file"; do > target=$(readlink "$file") > case "$target" in > /*) > file="$target" > ;; > *) > file="$(dirname "$file")/$target" > ;; > esac > done > else > echo "$myname: 'readlink' missing. Failed to follow symlink $1." 1>&2 > exit 1 > fi > else > file="$1" > fi Looks good, one minor tweak I suggest is having: file=$1 above that and using $file after that, so we refer to $1 only once (it's less obvious what $1 is than $file). And maybe exit early if readlink isn't available to avoid one indent: if ! command -v readlink >/dev/null 2>&1; then echo ... exit 1 fi ... continue with using readlink ... Simon