Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: "Metzger, Markus T" <markus.t.metzger@intel.com>
To: Jan Kratochvil <jan.kratochvil@redhat.com>
Cc: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>,
	"markus.t.metzger@gmail.com" <markus.t.metzger@gmail.com>
Subject: RE: [rfc 3/5] record: make it build again
Date: Mon, 11 Feb 2013 13:41:00 -0000	[thread overview]
Message-ID: <A78C989F6D9628469189715575E55B2307B78A04@IRSMSX102.ger.corp.intel.com> (raw)
In-Reply-To: <20130210221059.GC4819@host2.jankratochvil.net>

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-owner@sourceware.org] On Behalf Of Jan Kratochvil
> Sent: Sunday, February 10, 2013 11:11 PM

Thanks for your review!

[...]

> >  static void
> > +record_info (void)
> 
> Such functions should be called/renamed to record_full_info, they are specific
> for record-full.c and moreover other backends will have the same function.
> 
> You can rename everything, also record_ops, record_core_ops -> record_full_*.
> 
> GDB prevents using static names duplicated across files.  (Maybe it comes from
> the time before "ambiguous linespec" start to put breakpoints on all of them.)

I also renamed struct, macro, and variable names to be consistent. To make this
easier to review, I'm doing this renaming in a separate patch. It's quite a lot.

[...]

> > +  /* Deprecate the old version without "full" prefix.  */
> > +  c = add_alias_cmd ("restore", "full restore", class_obscure, 1,
> > +		     &record_cmdlist);
> > +  set_cmd_completer (c, filename_completer);
> > +  deprecate_cmd (c, "record full restore");
> 
> This (and all add_alias_cmd below) don't display the warning as discussed
> before.
> 
> I guess we can keep it as is as the missing warning is tracked
> 	deprecated_cmd_warning does not work for prefixed commands
> 	http://sourceware.org/bugzilla/show_bug.cgi?id=15104
> and the only command where it is most visible is "target record" which you
> have successfully workarounded in the patchset.

Sounds good. Thanks.

> a bit offtopic: 'git am' somehow broken on applicating the patchset (but later
> it went OK by hand), it would be easier to have it in a public GIT branch,
> possibly in
> 	http://sourceware.org/gdb/wiki/ArcherBranchManagement
> needing an account http://sourceware.org/cgi-bin/pdw/ps_form.cgi which you are
> going to get for the later check-in anyway; or you could use github or some
> such site.

Can I use my sourceware account also for Archer or do I need to request a new
account for it?

Is it OK to rebase an archer branch? I do this to incorporate review comments to
maintain a reviewable patch series.

I would still send patches, right? And they are still expected to apply on gdb's
Master, right?

The next patch I'm adding will rely on the btrace series plus the patches in this
series. It will not apply without btrace below, anymore. That's quite a big series
for adding a small patch. Would Archer help me, there?

How would I send patches so that people know what to review and at the
same time allow them to apply the patch and try the changes?

[...]

I interpret your replies to the other patches in the series such that you
approved http://sourceware.org/ml/gdb-patches/2013-02/msg00217.html,
http://sourceware.org/ml/gdb-patches/2013-02/msg00216.html, and
http://sourceware.org/ml/gdb-patches/2013-02/msg00212.html. I added
comments to the commit messages.

Regards,
Markus.
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052


  reply	other threads:[~2013-02-11 13:41 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-02-08 15:30 [rfc 0/5] record-btrace markus.t.metzger
2013-02-08 15:30 ` [rfc 1/5] target: add add_deprecated_target_alias markus.t.metzger
2013-02-10 22:10   ` Jan Kratochvil
2013-02-08 15:30 ` [rfc 5/5] record, disas: add record disassemble command markus.t.metzger
2013-02-10 22:11   ` Jan Kratochvil
2013-02-08 15:31 ` [rfc 3/5] record: make it build again markus.t.metzger
2013-02-10 22:11   ` Jan Kratochvil
2013-02-11 13:41     ` Metzger, Markus T [this message]
2013-02-11 14:15       ` Jan Kratochvil
2013-02-11 17:13         ` [draft patch] <unavailable> unwinder for btrace [Re: [rfc 3/5] record: make it build again] Jan Kratochvil
2013-02-11 21:24           ` Tom Tromey
2013-02-13  7:35           ` Metzger, Markus T
2013-02-13  7:58             ` Jan Kratochvil
2013-02-13  8:08               ` Metzger, Markus T
2013-03-27 18:09           ` Metzger, Markus T
2013-03-28 12:38             ` Markus Metzger
     [not found]               ` <20130328062747.GA27157@host2.jankratochvil.net>
2013-03-28 14:28                 ` Metzger, Markus T
2013-03-28 14:38                   ` Jan Kratochvil
2013-03-28 17:37                     ` Metzger, Markus T
2013-02-08 15:31 ` [rfc 4/5] record: default target methods markus.t.metzger
2013-02-10 22:11   ` Jan Kratochvil
2013-02-08 15:32 ` [rfc 2/5] record: split record markus.t.metzger
2013-02-10 22:11   ` Jan Kratochvil

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=A78C989F6D9628469189715575E55B2307B78A04@IRSMSX102.ger.corp.intel.com \
    --to=markus.t.metzger@intel.com \
    --cc=gdb-patches@sourceware.org \
    --cc=jan.kratochvil@redhat.com \
    --cc=markus.t.metzger@gmail.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