Re: [ARI] Remove all editCase warnings

Pedro Alves
2010-05-05T15:11:34Z

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