From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 84605 invoked by alias); 28 Oct 2016 19:39:58 -0000 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 Received: (qmail 84584 invoked by uid 89); 28 Oct 2016 19:39:57 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.2 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 spammy=retaining X-HELO: mailapp01.imgtec.com Received: from mailapp01.imgtec.com (HELO mailapp01.imgtec.com) (195.59.15.196) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 28 Oct 2016 19:39:56 +0000 Received: from HHMAIL01.hh.imgtec.org (unknown [10.100.10.19]) by Forcepoint Email with ESMTPS id D813AF51FD8D6; Fri, 28 Oct 2016 20:39:49 +0100 (IST) Received: from [10.20.78.230] (10.20.78.230) by HHMAIL01.hh.imgtec.org (10.100.10.21) with Microsoft SMTP Server id 14.3.294.0; Fri, 28 Oct 2016 20:39:52 +0100 Date: Fri, 28 Oct 2016 19:39:00 -0000 From: "Maciej W. Rozycki" To: Pedro Alves , Yao Qi CC: Subject: Re: [PATCH 06/13] Add enum for mips breakpoint kinds In-Reply-To: <781ba8eb-ba42-58e0-1ead-ae44d8a39409@redhat.com> Message-ID: References: <1472655965-12212-1-git-send-email-yao.qi@linaro.org> <1472655965-12212-7-git-send-email-yao.qi@linaro.org> <781ba8eb-ba42-58e0-1ead-ae44d8a39409@redhat.com> User-Agent: Alpine 2.00 (DEB 1167 2008-08-23) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" X-SW-Source: 2016-10/txt/msg00824.txt.bz2 Pedro, Yao -- I missed the MIPS parts previously, sorry. On Thu, 27 Oct 2016, Pedro Alves wrote: > > +/* Enum describing the different kinds of breakpoints. */ > > + > > +enum mips_breakpoint_kinds > > IMO that should be singular. Imagine you put one of these > in a variable. Like: > > enum mips_breakpoint_kinds kind; > > This would be more natural, IMO: > > enum mips_breakpoint_kind kind; Agreed. > > +{ > > + /* 16-bit MIPS16 mode breakpoint */ > > + MIPS_BP_KIND_16BIT_MIPS16 = 2, > > + /* 16-bit microMIPS mode breakpoint */ > > + MIPS_BP_KIND_16BIT_MICROMIPS = 3, > > + /* 32-bit standard MIPS mode breakpoint */ > > + MIPS_BP_KIND_32BIT = 4, > > + /* 32-bit microMIPS mode breakpoint */ > > + MIPS_BP_KIND_32BIT_MICROMIPS = 5, > > IMO a line break between these makes it much more readable. > > /* 16-bit MIPS16 mode breakpoint */ > MIPS_BP_KIND_16BIT_MIPS16 = 2, > > /* 16-bit microMIPS mode breakpoint */ > MIPS_BP_KIND_16BIT_MICROMIPS = 3, > > etc. Indeed. Also a full stop and two spaces at the end are due. Also how about calling them: MIPS_BP_KIND_MIPS16 MIPS_BP_KIND_MICROMIPS16 MIPS_BP_KIND_MIPS32 MIPS_BP_KIND_MICROMIPS32 to make the names a bit shorter though still retaining the meaning? Maciej