From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5333 invoked by alias); 3 Dec 2011 08:20:30 -0000 Received: (qmail 5191 invoked by uid 22791); 3 Dec 2011 08:20:02 -0000 X-SWARE-Spam-Status: No, hits=-2.2 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW X-Spam-Check-By: sourceware.org Received: from mail-ww0-f43.google.com (HELO mail-ww0-f43.google.com) (74.125.82.43) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sat, 03 Dec 2011 08:19:47 +0000 Received: by wgbds11 with SMTP id ds11so2361049wgb.12 for ; Sat, 03 Dec 2011 00:19:46 -0800 (PST) MIME-Version: 1.0 Received: by 10.216.26.203 with SMTP id c53mr207213wea.88.1322900386393; Sat, 03 Dec 2011 00:19:46 -0800 (PST) Received: by 10.180.96.72 with HTTP; Sat, 3 Dec 2011 00:19:46 -0800 (PST) In-Reply-To: References: <998639.46560.qm@web112516.mail.gq1.yahoo.com> <321260.58442.qm@web112504.mail.gq1.yahoo.com> <1316327455.23344.YahooMailNeo@web112509.mail.gq1.yahoo.com> <1316404058.27177.YahooMailNeo@web112502.mail.gq1.yahoo.com> <1318650316.91503.YahooMailNeo@web112508.mail.gq1.yahoo.com> Date: Sat, 03 Dec 2011 08:20:00 -0000 Message-ID: Subject: Re: [PATCH] arm reversible : From: oza Pawandeep To: Tom Tromey Cc: =?ISO-8859-1?Q?Petr_Hluz=EDn?= , paawan oza , "gdb-patches@sourceware.org" , chandra krishnappa Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable 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: 2011-12/txt/msg00068.txt.bz2 Hi Tom, Thanks for the comments. 1) I have been trying to fix all formatting, it looks better now and fixed all comments you had. I am sorry that on formatting both of our efforts are going; excuse me as every time mailer spoils the things; probably copy from xemacs editor to chrome/firefox in plain text and the other mailer gets it. though I am not sure. 2) I could not write changelog entry because I have been getting commments, and moreover I was not sure whether I should get changelog entry from latest gdb 7.3 or latest current working version of gdb ? I think later is true in my understanding. I will include changelog this time; sorry for having you point it out in all reviews. 3) will post the latest patch, with possible correction of formatting issues; and will include changelog from current branch. Regards, Oza. On Sat, Dec 3, 2011 at 12:06 AM, Tom Tromey wrote: >>>>>> "oza" =3D=3D oza Pawandeep writes: > > oza> please find the updated patch; I have tried my level best to take ca= re > oza> the most to get formatting right. > oza> And also, all extra parentheses removed. > > Thanks. > > oza> please have a look. =A0(hope mailer does not do anything) > > Your mailer is still screwed up. > > You didn't write a ChangeLog entry or a NEWS entry. > Please do this. =A0I have asked at least once now. =A0These are just > baseline things for getting patches into gdb; if you don't respond to > these requests it makes me tend to prioritize other patches over yours. > FYI. > > oza> + =A0 =A0 =A0/* if this insn has fallen into extension space > oza> + =A0 =A0 =A0 =A0 then we need not decode it anymore. =A0*/ > oza> + =A0 =A0 =A0if (ret !=3D -1 && !INSN_RECORDED(arm_record)) > oza> + =A0 =A0 =A0 =A0{ > oza> + =A0 =A0 =A0 =A0 =A0arm_handle_insn[insn_id] (arm_record); > > Still not checking the return value. > > oza> +/* cleans up local record registers and memory allocations. =A0*/ > > Should start with a capital letter. > Blank line between comment and function. > > oza> +void deallocate_reg_mem(insn_decode_record *record) > > Newline after void. > Space before open paren. > > oza> +{ > oza> + =A0if (record->arm_regs) > oza> + =A0 =A0xfree (record->arm_regs); > oza> + =A0if (record->arm_mems) > oza> + =A0 =A0xfree (record->arm_mems); > oza> +} > > You can call xfree on a NULL pointer. =A0Just remove those ifs. > > oza> + =A0 =A0 =A0 =A0 =A0for (no_of_rec =3D 0; no_of_rec < arm_record.me= m_rec_count; > oza> no_of_rec++) > oza> + =A0 =A0 =A0 =A0 =A0{ > > Brace indentation looks wrong. > > There are still other minor formatting issues. =A0I saw 21 places where > there is a comma at the start of a line, please fix all of these. > I think there were some other problems, too, but at this point I have > stopped caring. > > Tom