From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22870 invoked by alias); 4 Dec 2011 14:46:07 -0000 Received: (qmail 22862 invoked by uid 22791); 4 Dec 2011 14:46:06 -0000 X-SWARE-Spam-Status: No, hits=-1.6 required=5.0 tests=AWL,BAYES_00,TW_SB X-Spam-Check-By: sourceware.org Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sun, 04 Dec 2011 14:45:49 +0000 Received: from nat-jpt.mentorg.com ([192.94.33.2] helo=PR1-MAIL.mgc.mentorg.com) by relay1.mentorg.com with esmtp id 1RXDK8-0002JU-FG from Yao_Qi@mentor.com ; Sun, 04 Dec 2011 06:45:48 -0800 Received: from [127.0.0.1] ([172.16.63.104]) by PR1-MAIL.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.1830); Sun, 4 Dec 2011 23:45:46 +0900 Message-ID: <4EDB877B.2050903@codesourcery.com> Date: Sun, 04 Dec 2011 14:46:00 -0000 From: Yao Qi User-Agent: Mozilla/5.0 (X11; Linux i686; rv:7.0.1) Gecko/20110930 Thunderbird/7.0.1 MIME-Version: 1.0 To: oza Pawandeep CC: =?UTF-8?B?UGV0ciBIbHV6w61u?= , Tom Tromey , paawan oza , "gdb-patches@sourceware.org" , chandra krishnappa Subject: Re: [PATCH] arm reversible : References: <998639.46560.qm@web112516.mail.gq1.yahoo.com> <4EDAD0EF.20405@codesourcery.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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/msg00098.txt.bz2 On 12/04/2011 07:32 PM, oza Pawandeep wrote: > Updated patch contains all the 3 review comments fixed (Tom, Petr and Yao). > Change log is elaborated; I am looking forwarding to adding more > details if need. seeking for your feedback on the same. > The format of ChangeLog looks odd to me. Some comments below, > diff -urN arm_orig/ChangeLog arm_new/ChangeLog > --- arm_orig/ChangeLog 2011-12-03 18:05:04.000000000 +0530 > +++ arm_new/ChangeLog 2011-12-04 16:45:00.000000000 +0530 > @@ -1,3 +1,65 @@ > +2011-12-03 Oza Pawandeep > + > + * arm-linux-tdep.c: arm_linux_init_abi modified to include > + arm reversible debugging feature. > + registered arm_process_record to gdb_arch > + syscall pointer initialization. It can be * arm-linux-tdep.c (arm_linux_init_abi): Call set_gdbarch_process_record. Initialize `arm_swi_record' field. Please care about the format of changelog entry, usually it is like this, [tab] * file-you-modified.c (function-you-modified): What is your [tab] Change. Note that we write "what is changed" in ChangeLog, instead of "why is changed". > + > + * arm-tdep.c: arm-reversible-debugging implementation. > + newly added functions are as follows. > + > + > arm_process_record: handles basic initialization and record > + summarisation, decodes basic insn ids, on which it hands over > + controls to decode_insn. So, we don't need so much details in changelog like this, we can write it in this way, * arm-tdep.c (arm_process_record): New. There are still many new added functions, and I think they can documented in ChangeLog in the same way. > + > deallocate_reg_mem : clean up function > + > decode_insn: Decodes arm/thumb insn and calls appropriate > + decoding routine to record the change. > + > thumb_record_branch: branch insn reording (thumb) > + > thumb_record_ldm_stm_swi: load, store and sycall insn > + recoding (thumb) > + > thumb_record_misc: misc insn recording (thumb) > + > thumb_record_ld_st_stack: store and stack insn recording (thumb) > + > thumb_record_ld_st_imm_offset: load, store with immediate offset > + insn recording (thumb) > + > thumb_record_ld_st_reg_offset: load, store with register offset > + recording (thumb) > + > thumb_record_add_sub_cmp_mov: addition, subtractation, compare > + and move insn recording (thumb) > + > thumb_record_shift_add_sub: shift, add and sub insn recording > + (thumb) > + > arm_record_coproc_data_proc: coprocessor and data processing > + recording (partially implemented) (arm) > + > arm_record_coproc: coprocessor insn recording > + (partially implemented) (arm) > + > arm_record_b_bl: branch insn recording (arm) > + > arm_record_ld_st_multiple: load and store multiple insn recording > + (arm) > + > arm_record_ld_st_reg_offset: load and store reg offset recording > + (arm) > + > arm_record_ld_st_imm_offset: load and store immediate offset > + recording (arm) > + > arm_record_data_proc_imm: data processing insn recording (arm) > + > arm_record_data_proc_misc_ld_str: data processing, misc, load and > + store insn recording (arm) > + > arm_record_extension_space:arm extension space insn recording > + (arm) > + > arm_record_strx: str(X) type insn recording (arm) > + > sbo_sbz: checks for mendatory sbo and sbz fields in insn, > + > + added new data structures: > + > insn_decode_record_t: local record structure which contains insn's > + record, which includes both reg and memory. > + > + REG_ALLOC and MEM_ALLOC macros takes care of actual memory allocation > + in local record which is finally processed by arm_rocess_record. > + > + * arm-tdep.h: arm-reversible data structures > + > + > modified gdbarch_tdep: added member (function pointer) arm_swi_record > + which is supposed to be recording system calls This can be written in this way, * arm-tdep.h (struct gdbarch_tdep): New field `arm_swi_record'. > + > arm_process_record externed. > + > + Again, reading other existing changelog entries must be helpful for you to write your own in a correct way/format. :) -- Yao (齐尧)