This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

RE: [ARI] Remove all editCase warnings


> -----Message d'origine-----
> De?: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Pedro Alves
> Envoyé?: Wednesday, May 05, 2010 1:44 AM
> À?: gdb-patches@sourceware.org
> Cc?: Pierre Muller
> Objet?: Re: [ARI] Remove all editCase warnings
> 
> On Wednesday 05 May 2010 00:25:48, Pierre Muller wrote:
> >         ARI: Fix editCase.
> >         * ada-lang.c (ada_remove_Xbn_suffix): Rename to...
> >         (ada_remove_xbn_suffix): ...this.
> 
> The function really removes "X", not "x".

  You are right.
 
> >         * addrmap.c (splay_compare_CORE_ADDR_ptr): Rename to...
> >         (splay_compare_core_addr_ptr): ...this.
> 
> errr.  Don't care.

  I also don't really care...
 
> >         * 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);
  
  
 
> >         * ia64-tdep.c (slotN_contents): Rename to...
> >         (slotn_contents): ...this.
> >         (replace_slotN_contents): Rename to...
> >         (replace_slotn_contents): ...this.
> 
> No comments.  But I bet whoever wrote this was well aware
> of our coding conventions and still chose an uppercase N.

  That's 
> 
> >         * remote.c (set_remote_protocol_Z_packet_cmd): Rename to...
> >         (set_remote_protocol_z_packet_cmd): ...this.
> >         (show_remote_protocol_Z_packet_cmd): Rename to...
> >         (show_remote_protocol_z_packet_cmd): ...this.
> >         (store_register_using_P): Rename to...
> >         (store_register_using_p): ...this.
> >         (store_register_using_G): Rename to...
> >         (store_register_using_g): ...this.
> >         (remote_store_registers): Adapt to name changes above.
> >         (watchpoint_to_Z_packet): Rename to...
> >         (watchpoint_to_z_packet): ...this.
> 
> I'd rather not.  Z, z, P, G, g, are all distinct packets, uppercase
> having the opposite effect of lowercase.  Example, you really store
> with G, not g, which is for fetches (note the `fetch_registers_using_g'
> function).

   Here I completely agree, I forgot that there were
lowercase g and z packets.
 
> >         * sol-thread.c (ps_lgetLDT): Add ARI comment for editCase.
> >         Adapt to name change in procfs.c source.
> 
> I'd bet this spelling was the reason for the `procfs_find_LDT_entry'
> spelling in procfs.c.  ps_lgetLDT is the main caller
> of `procfs_find_LDT_entry'. 
> >         * windows-nat.c (_initialize_check_for_gdb_ini): Add ARI
> >         comment to all windows API replacement functions.
> 
> 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.
  
  This way, we could either decide that the mixed case is not
necessary and lowercase the function name, or add an ARI comment to accept
that exception.

  I would tend to agree that adding an ARI comment is probably justified on
most of these 18 functions.
  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.

  - 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

  Please tell me how I should proceed.

Pierre



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]