From: Simon Marchi <simon.marchi@polymtl.ca>
To: Lancelot SIX <lsix@lancelotsix.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH v3] [PR gdb/27614] gdb-add-index fails on symlinks.
Date: Sun, 28 Mar 2021 13:51:02 -0400 [thread overview]
Message-ID: <2cba4c43-f085-ace3-d3f2-5f30bff426f3@polymtl.ca> (raw)
In-Reply-To: <YGC1P20n0c+uqZvb@Plymouth>
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 <file>`?
>>
>>> 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
prev parent reply other threads:[~2021-03-28 17:52 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-27 17:27 Lancelot SIX
2021-03-28 16:11 ` Simon Marchi
2021-03-28 16:56 ` Lancelot SIX
2021-03-28 17:51 ` Simon Marchi [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2cba4c43-f085-ace3-d3f2-5f30bff426f3@polymtl.ca \
--to=simon.marchi@polymtl.ca \
--cc=gdb-patches@sourceware.org \
--cc=lsix@lancelotsix.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).