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