Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Pedro Alves <palves@redhat.com>
To: Jan Kratochvil <jan.kratochvil@redhat.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [patch 2/3] attach-fail-reasons: Say more than ptrace: Operation not permitted.
Date: Thu, 08 Mar 2012 13:55:00 -0000	[thread overview]
Message-ID: <4F58BA4A.9090903@redhat.com> (raw)
In-Reply-To: <20120306061710.GB24004@host2.jankratochvil.net>

On 03/06/2012 06:17 AM, Jan Kratochvil wrote:

> +void
> +linux_ptrace_attach_warnings (pid_t pid)
> +{
> +  pid_t tracerpid;
> +
> +  tracerpid = linux_proc_get_tracerpid (pid);


...

> +extern pid_t linux_proc_get_tracerpid (int lwpid);
..

Would be neater with `pid_t lwpid'.

I agree this is a useful addition.

linux_ptrace_attach_warnings has a tiny race, in that by the time you open /proc/pid/... you
may be looking at a different process.  There's no way to fix that, I think, and this
will be right in the large majority of the cases, so we can just live with it, IMO.

You may have considered the following already, but I haven't seen it mentioned.  I think it
may be better if the new warning strings are actually part of the thrown error string:

  - for example, frontends that display the error to the user as a dialog
    box or similar show the whole warning+error automatically, instead of having the warning
    get lost in the console noise (if it's visible at all).
  - It's perhaps better for gdbserver as well, as we can forward the whole error string
    to GDB so that GDB shows it to the user (the `E.' thing) (although we could forward warnings as
    well with the O packet, though we don't do that presently).
  - If some code in GDB, or most likely a python script, wraps "attach", and catches the error,
    it presumably will also want to capture the warnings.  Having everything in the
    exception seems cleaner.

(Note this is not an objection.  Your patch is useful as it is.)

-- 
Pedro Alves


  reply	other threads:[~2012-03-08 13:55 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-03-06  6:17 Jan Kratochvil
2012-03-08 13:55 ` Pedro Alves [this message]
2012-03-13  9:43   ` Jan Kratochvil
2012-03-13 10:33     ` Pedro Alves
2012-03-13 13:55       ` Jan Kratochvil
2012-03-13 14:35         ` Pedro Alves
2012-03-13 14:50           ` Jan Kratochvil
2012-03-13 15:03           ` [commit] " Jan Kratochvil
2012-03-14 15:12         ` Tom Tromey

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=4F58BA4A.9090903@redhat.com \
    --to=palves@redhat.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