Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: "Maciej W. Rozycki" <macro@codesourcery.com>
To: Jan Kratochvil <jan.kratochvil@redhat.com>
Cc: Tom Tromey <tromey@redhat.com>, <gdb-patches@sourceware.org>,
	Sergio Durigan <sdurigan@redhat.com>
Subject: Re: [RFA] MIPS16 FP manual call/return fixes
Date: Wed, 16 May 2012 14:45:00 -0000	[thread overview]
Message-ID: <alpine.DEB.1.10.1205161536010.11227@tp.orcam.me.uk> (raw)
In-Reply-To: <20120514115338.GA14436@host2.jankratochvil.net>

On Mon, 14 May 2012, Jan Kratochvil wrote:

> > strictly speaking we can keep the structure flat if 
> > we decide that complicating it is not worth the saving.
> 
> Technically yes but it is more readable to know this field is valid/used only
> with this subclass.  And it is better not just to rely on unstructured
> comments content.  And to give the field name its real meaning ("func_addr")
> and not just a generic name of overload ("related").  A good example of these
> overloaded generic fields without subclassing is main_type which is such as
> mess I still have problems to fully understand it.

 Yes, I can't disagree, common sense applies.

> >  Actually, I relied on the lack of compilation errors so far; it seems 
> > that while we do have -Wall -Werror, we have -Wno-unused as well which 
> > implies -Wno-unused-variable and has defeated my assumptions.  Any 
> > particular reason why we disable this warning?
> 
> Because there are now many such unused-variable cases needing to be fixed,
> there were recent threads about it (see subject /unused/) by Sergio.
> It looks still not all the cases are fixed to enable the warnings.

 Hmm, fair enough, I wonder if we could take -Wno-unused out in the 
maintainer's mode so as to prompt people to fix their bugs?

> > > >  	      b->type = bp_gnu_ifunc_resolver;
> > > 
> > > Empty line before a comment according to GDB Coding Standards.
> > > 
> > > > +	      /* Remember the resolver's address for use by the return
> [...]
> >  I believe this requirement applies to function and variable/type/etc. 
> > definitions only and I haven't seen this style applied inline in many 
> > places (including the original comment above, actually).  Are you sure?
> 
> I find it more readable with the empty line even in this case.
> You are right there are too many of such cases without empty line in GDB.
> Therefore I do not mind, check it in either way.

 I think common sense applies again, I often find these extra empty lines 
disturbing with small comments, but larger ones definitely do need this 
extra space so that they do not obscure actual code.

 Anyway, 48 hours have passed with no further comments, so I have 
committed this change now, as posted.  If anything that was missed by 
chance pops out, then we can address it with a follow-up change.

 Thanks to everybody involved.

  Maciej


      reply	other threads:[~2012-05-16 14:45 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-04-23 13:30 Maciej W. Rozycki
2012-04-26 19:20 ` Tom Tromey
2012-04-26 22:23   ` Maciej W. Rozycki
2012-04-27 15:16     ` Maciej W. Rozycki
2012-04-27 15:28       ` Tom Tromey
2012-04-26 19:33 ` Tom Tromey
2012-04-30 23:45   ` Maciej W. Rozycki
2012-05-01 14:05     ` Jan Kratochvil
2012-05-01 17:22       ` Maciej W. Rozycki
2012-05-02 21:28         ` Jan Kratochvil
2012-05-08 14:30           ` Maciej W. Rozycki
2012-05-12 19:38             ` Jan Kratochvil
2012-05-14  9:12               ` Maciej W. Rozycki
2012-05-14 11:54                 ` Jan Kratochvil
2012-05-16 14:45                   ` Maciej W. Rozycki [this message]

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=alpine.DEB.1.10.1205161536010.11227@tp.orcam.me.uk \
    --to=macro@codesourcery.com \
    --cc=gdb-patches@sourceware.org \
    --cc=jan.kratochvil@redhat.com \
    --cc=sdurigan@redhat.com \
    --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