Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Joel Brobecker <brobecker@adacore.com>
To: Tom Tromey <tromey@redhat.com>
Cc: gdb-patches@sourceware.org
Subject: Re: FYI: add some breakpoint setter methods
Date: Mon, 31 Jan 2011 02:42:00 -0000	[thread overview]
Message-ID: <20110131021033.GC2384@adacore.com> (raw)
In-Reply-To: <m37hdplpj5.fsf@fleche.redhat.com>

> As an aside, it seems to me that the breakpoint observers should
> probably take a 'struct breakpoint *' as an argument, rather than a
> breakpoint number.  This would be more efficient: I think most users
> will want to have access to the actual breakpoint anyhow, and this would
> avoid a search through all breakpoints for the number.  Perhaps I will
> implement this.

Seems like a good idea. I had a discussion like that with one of the
contributors a while ago, trying to convince him to use a breakpoint
struct instead of the breakpoint number in his observer. I think it was
for annotations.  In any case, he argued that this was sufficient for
his needs so far, so I let it go, but it came back to "bite" me later on
("bite" <=> I had to change it to use struct breakpoint).

> 2011-01-27  Tom Tromey  <tromey@redhat.com>
> 
> 	* infcmd.c (finish_backward): Use breakpoint_set_silent.
> 	* python/py-breakpoint.c (bppy_set_silent): Use
> 	breakpoint_set_silent.
> 	(bppy_set_thread): Use breakpoint_set_thread.
> 	(bppy_set_task): Use breakpoint_set_task.
> 	* breakpoint.h (breakpoint_set_silent, breakpoint_set_thread)
> 	(breakpoint_set_task): Declare.
> 	(make_breakpoint_silent): Remove.
> 	* breakpoint.c (breakpoint_set_silent): New function.
> 	(breakpoint_set_thread): Likewise.
> 	(breakpoint_set_task): Likewise.
> 	(make_breakpoint_silent): Remove.

Just a nit I noticed:

> +/* Set the internal `silent' flag on the breakpoint.  Note that this
> +   is not the same as the "silent" that may appear in the breakpoint's
> +   commands.  */
> +
> +void
> +breakpoint_set_silent (struct breakpoint *b, int silent)
> +{
> +  int old_silent = b->silent;
> +  b->silent = silent;

Missing empty line after variable declaration (it's happening multiple
times throughout the patch).

-- 
Joel


  reply	other threads:[~2011-01-31  2:10 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-01-28  2:40 Tom Tromey
2011-01-31  2:42 ` Joel Brobecker [this message]
2011-01-31 11:04   ` Vladimir Prus
2011-01-31 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=20110131021033.GC2384@adacore.com \
    --to=brobecker@adacore.com \
    --cc=gdb-patches@sourceware.org \
    --cc=tromey@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