Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Joel Brobecker <brobecker@adacore.com>
To: Jan Kratochvil <jan.kratochvil@redhat.com>
Cc: gdb-patches@sourceware.org
Subject: Re: ping: [patch 5/6] testsuite: Fix prelink-support.exp without prelink installed
Date: Tue, 29 Jun 2010 22:09:00 -0000	[thread overview]
Message-ID: <20100629220904.GV2595@adacore.com> (raw)
In-Reply-To: <20100609150954.GE7183@host0.dyn.jankratochvil.net>

> gdb/testsuite/
> 2010-03-29  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	Cope with missing /usr/sbin/prelink.
> 	* lib/prelink-support.exp (prelink_no):
> 	<result == 1 && $output is "no such file or directory">: New.
> 	(prelink_yes): Likewise.  Return on failed prelink_no.

This should be OK.  I'm just concerned about matching the output
from the prelink command, since it is assuming that it is going to
be in English. Do you know if we force the language in the testsuite?

I'm just wondering whether a more straightforward/brutal approach might
be simpler and as effective: What if we searched for prelink in the PATH
and skipped the entire test if not found?

> +	# Without prelink at lest verify all the binaries do not contain the
> +	# ".gnu.prelink_undo" section so that they are not already prelinked.

Missing comma, and spell-o:

# Without prelink, at le*a*st verify *that* all the binaries do not
# contain the  ".gnu.prelink_undo" section (which would mean that they
# have already been prelinked).

> +	# While we could check if $arg is already prelinked (as if someone
> +	# uninstalls prelink after having the system ld.so prelinked) we cannot
> +	# change its prelinked address.  Therefore rather skip the test.

I'm not a big fan of "as if [...]" in the case of documentation. It's OK
when spoken, but it's a little too subtle for written documentation.
I suggest something plainer:

# We could not find prelink.  We could check whether $args is already
# prelinked but we don't, because:
#   - It is unlikely that someone uninstalls prelink after having
#     prelinked the system ld.so;
#   - We still cannot change its prelinked address.
# Therefore, we just skip the test.

-- 
Joel


  reply	other threads:[~2010-06-29 22:09 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-03-29 16:18 Jan Kratochvil
2010-06-09 15:10 ` ping: " Jan Kratochvil
2010-06-29 22:09   ` Joel Brobecker [this message]
2010-07-04 10:19     ` Jan Kratochvil
2010-07-05 17:40       ` Joel Brobecker
2010-07-05 18:13         ` Jan Kratochvil
2010-07-19 15:06       ` Pedro Alves
2010-07-19 15:43         ` Daniel Jacobowitz
2010-07-28 14:26           ` Disable i18n when running the testsuite (Re: ping: [patch 5/6] testsuite: Fix prelink-support.exp without prelink installed) Pedro Alves
2010-07-28 17:36             ` Joel Brobecker
2010-07-29 14:20               ` Disable i18n when running the testsuite Pedro Alves

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=20100629220904.GV2595@adacore.com \
    --to=brobecker@adacore.com \
    --cc=gdb-patches@sourceware.org \
    --cc=jan.kratochvil@redhat.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