Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Pedro Alves <pedro@codesourcery.com>
To: "Pierre Muller" <pierre.muller@ics-cnrs.unistra.fr>
Cc: gdb-patches@sourceware.org
Subject: Re: [ARI] Remove all editCase warnings
Date: Wed, 05 May 2010 15:11:00 -0000	[thread overview]
Message-ID: <201005051611.09790.pedro@codesourcery.com> (raw)
In-Reply-To: <000901caec31$d109e640$731db2c0$@muller@ics-cnrs.unistra.fr>

On Wednesday 05 May 2010 10:03:23, Pierre Muller wrote:
>  
> > >         * hppa-tdep.c (hppa_extract_5R_store): Rename to...
> > >         (hppa_extract_r_store): ...this.
> > 
> > I don't know this code at all, but I wouldn't be suprised
> > if the 5r vs 5R had meaning to someone knowing hppa.  (there's
>   You are also most probably right.
> > a "5r" variant of this function above the "5R" variant.)  E.g.,:
>   I view that, that is why I used 5_r to replace 5R and not simply 5r... 
> >  int hppa_extract_5_load (unsigned int);
> >  unsigned hppa_extract_5R_store (unsigned int);
> >  unsigned hppa_extract_5r_store (unsigned int);

IMO, that's really much more error prone:

 unsigned hppa_extract_5_r_store (unsigned int);
 unsigned hppa_extract_5r_store (unsigned int);

Even easier to confuse the two...  Unless we have better, self
describing names (which would probably require someone knowing
hppa stepping in) let's leave this untouched.

> > Honestly, I think this rule is one of those that generates
> > work for not such a great reason.  IMO, code review enough catches
> > all the bad cases before they get to the tree.
>  
>   My idea was to get rid of all issues so that I could mark that
> rule as a regression, and that it would thus get a much greater
> impact if new code would use such mixed case function names.

What impact do you expect here?  Speaking for myself, if I review
a patch, and I see something uppercased, and I don't think it fits
right in the coding conventions, I'll say so during review.  If I feel
the uppercase is okay, I'll either remember to ask to uglify the
sources by adding an ARI expection marker, or, I'll more likely
forget it, and we'll get a nag email from the ARI script, causing
us have to do unproductive work to fix it.  Neither of the last two
options seems attractive to me.

>   The only ones I am not sure about are:
>   - splay_compare_CORE_ADDR_ptr in addrmap.c
> there are numerous other functions refereeing to CORE_ADDR type without
> using uppercase:
> $ grep "^[a-z_0-9]*core_addr" *
> arch-utils.c:core_addr_lessthan (CORE_ADDR lhs, CORE_ADDR rhs)
> arch-utils.c:core_addr_greaterthan (CORE_ADDR lhs, CORE_ADDR rhs)
> arch-utils.c:core_addr_identity (struct gdbarch *gdbarch, CORE_ADDR addr)
> proc-service.c:ps_addr_to_core_addr (psaddr_t addr)
> proc-service.c:core_addr_to_ps_addr (CORE_ADDR addr)
> ui-out.c:ui_out_field_core_addr (struct ui_out *uiout,
> utils.c:core_addr_to_string (const CORE_ADDR addr)
> utils.c:core_addr_to_string_nz (const CORE_ADDR addr)
> utils.c:string_to_core_addr (const char *my_string)
>   
>   So I would like to eliminate that one.

Okay, fine with me.

>   - the other two are
> proc_get_LDT_entry and procfs_find_LDT_entry in procfs.c
> 
> these two functions are only called by 
> ps_lgetLDT inside sol-thread.c, which is, according to the comment
> required by libthread_db solaris library. 
>   But I do not know if this also justifies propagating the uppercase use
> to the called functions in procfs.c

In this case, between having a distracting ARI marker in the code,
and lowercasing the function name, I prefer the latter.

-- 
Pedro Alves


  parent reply	other threads:[~2010-05-05 15:11 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-05-04 23:25 Pierre Muller
2010-05-04 23:32 ` Stan Shebs
2010-05-04 23:44 ` Pedro Alves
2010-05-04 23:51   ` Michael Snyder
2010-05-05  9:03   ` Pierre Muller
2010-05-05 14:54     ` Joel Brobecker
2010-05-05 15:11     ` Pedro Alves [this message]
2010-05-05 16:09       ` Pierre Muller
2010-05-05 16:55         ` Joel Brobecker
2010-05-05 17:15           ` Stan Shebs
2010-05-05 21:03             ` Pierre Muller
2010-05-06 18:10               ` Joel Brobecker
2010-05-05 18:34           ` Daniel Jacobowitz
2010-05-05 18:37             ` Joel Brobecker

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=201005051611.09790.pedro@codesourcery.com \
    --to=pedro@codesourcery.com \
    --cc=gdb-patches@sourceware.org \
    --cc=pierre.muller@ics-cnrs.unistra.fr \
    /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