Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: "Pierre Muller" <pierre.muller@ics-cnrs.unistra.fr>
To: "'Pedro Alves'" <pedro@codesourcery.com>, <gdb-patches@sourceware.org>
Subject: RE: [ARI] Remove all editCase warnings
Date: Wed, 05 May 2010 09:03:00 -0000	[thread overview]
Message-ID: <000901caec31$d109e640$731db2c0$@muller@ics-cnrs.unistra.fr> (raw)
In-Reply-To: <201005050044.28991.pedro@codesourcery.com>

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



  parent reply	other threads:[~2010-05-05  9:03 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 [this message]
2010-05-05 14:54     ` Joel Brobecker
2010-05-05 15:11     ` Pedro Alves
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='000901caec31$d109e640$731db2c0$@muller@ics-cnrs.unistra.fr' \
    --to=pierre.muller@ics-cnrs.unistra.fr \
    --cc=gdb-patches@sourceware.org \
    --cc=pedro@codesourcery.com \
    /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