This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

RE: [rfc 3/5] record: make it build again


> -----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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]