Re: [ARI] Remove all editCase warnings

[Pedro Alves <pedro at codesourcery dot com>]

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