This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [ARI] Remove all editCase warnings
- From: Pedro Alves <pedro at codesourcery dot com>
- To: "Pierre Muller" <pierre dot muller at ics-cnrs dot unistra dot fr>
- Cc: gdb-patches at sourceware dot org
- Date: Wed, 5 May 2010 16:11:09 +0100
- Subject: Re: [ARI] Remove all editCase warnings
- References: <005b01caebe1$2183b890$648b29b0$@muller@ics-cnrs.unistra.fr> <201005050044.28991.pedro@codesourcery.com> <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