From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2210 invoked by alias); 5 May 2010 15:11:34 -0000 Received: (qmail 2198 invoked by uid 22791); 5 May 2010 15:11:33 -0000 X-SWARE-Spam-Status: No, hits=-2.2 required=5.0 tests=AWL,BAYES_00,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 05 May 2010 15:11:22 +0000 Received: (qmail 17879 invoked from network); 5 May 2010 15:11:17 -0000 Received: from unknown (HELO orlando.localnet) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 5 May 2010 15:11:17 -0000 From: Pedro Alves To: "Pierre Muller" Subject: Re: [ARI] Remove all editCase warnings Date: Wed, 05 May 2010 15:11:00 -0000 User-Agent: KMail/1.12.2 (Linux/2.6.31-20-generic; KDE/4.3.2; x86_64; ; ) Cc: gdb-patches@sourceware.org References: <005b01caebe1$2183b890$648b29b0$@muller@ics-cnrs.unistra.fr> <201005050044.28991.pedro@codesourcery.com> <000901caec31$d109e640$731db2c0$@muller@ics-cnrs.unistra.fr> In-Reply-To: <000901caec31$d109e640$731db2c0$@muller@ics-cnrs.unistra.fr> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201005051611.09790.pedro@codesourcery.com> X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2010-05/txt/msg00097.txt.bz2 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