From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 31106 invoked by alias); 25 Jul 2009 22:20:56 -0000 Received: (qmail 31097 invoked by uid 22791); 25 Jul 2009 22:20:55 -0000 X-SWARE-Spam-Status: No, hits=-2.4 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from smtp-outbound-1.vmware.com (HELO smtp-outbound-1.vmware.com) (65.115.85.69) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sat, 25 Jul 2009 22:20:49 +0000 Received: from jupiter.vmware.com (mailhost5.vmware.com [10.16.68.131]) by smtp-outbound-1.vmware.com (Postfix) with ESMTP id 7254813019; Sat, 25 Jul 2009 15:20:47 -0700 (PDT) Received: from [10.20.94.141] (msnyder-server.eng.vmware.com [10.20.94.141]) by jupiter.vmware.com (Postfix) with ESMTP id 69272DC3AA; Sat, 25 Jul 2009 15:20:47 -0700 (PDT) Message-ID: <4A6B83A8.8030505@vmware.com> Date: Sun, 26 Jul 2009 07:46:00 -0000 From: Michael Snyder User-Agent: Thunderbird 1.5.0.12 (X11/20080411) MIME-Version: 1.0 To: Hui Zhu CC: Mark Kettenis , gdb-patches ml Subject: Re: [RFA/RFC Prec] Add Linux AMD64 process record support second version, (64 bits system call support) 2/3 References: <4A5A82F9.5050503@vmware.com> <4A611BB8.2050808@vmware.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit 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: 2009-07/txt/msg00634.txt.bz2 Hui, I have just a few more small issues with this patch: > linux-record.c | 2570 ++++++++++++++++++++++++++++++------------------------ [...] > + case RECORD_SYS_ACCEPT: > + case RECORD_SYS_GETSOCKNAME: > + case RECORD_SYS_GETPEERNAME: > + { > + regcache_raw_read_unsigned (regcache, tdep->arg2, > + &tmpulongest); > + if (tmpulongest) > + { > + gdb_byte *a = alloca (tdep->size_ulong * 2); > + int addrlen; > + gdb_byte *addrlenp; > + tmpulongest += tdep->size_ulong; > + if (target_read_memory ((CORE_ADDR) tmpulongest, a, > + tdep->size_ulong * 2)) > + { > + if (record_debug) > + fprintf_unfiltered (gdb_stdlog, > + "Process record: error reading " > + "memory at addr = 0x%s len = %d.\n", > + OUTPUT_REG (tmpulongest, tdep->arg2), > + tdep->size_ulong * 2); > + return -1; > + } > + if (record_linux_sockaddr (regcache, tdep, > + extract_unsigned_integer > + (a, tdep->size_ulong, byte_order), > + extract_unsigned_integer > + (a + tdep->size_ulong, > + tdep->size_ulong, byte_order))) Could you use a couple of temporary variables for extract_unsigned_integer here? > + case RECORD_SYS_RECVFROM: > + regcache_raw_read_unsigned (regcache, tdep->arg2, > + &tmpulongest); > + if (tmpulongest) > + { > + gdb_byte *a = alloca (tdep->size_ulong * 2); > + int addrlen; > + gdb_byte *addrlenp; > > + tmpulongest += tdep->size_ulong * 4; > + if (target_read_memory ((CORE_ADDR) tmpulongest, a, > + tdep->size_ulong * 2)) > + { > + if (record_debug) > + fprintf_unfiltered (gdb_stdlog, > + "Process record: error reading " > + "memory at addr = 0x%s len = %d.\n", > + OUTPUT_REG (tmpulongest, tdep->arg2), > + tdep->size_ulong * 2); > + return -1; > + } > + if (record_linux_sockaddr (regcache, tdep, > + extract_unsigned_integer > + (a, tdep->size_ulong, byte_order), > + extract_unsigned_integer > + (a + tdep->size_ulong, > + tdep->size_ulong, byte_order))) And here? > + case RECORD_SYS_RECVMSG: > + { > + gdb_byte *a = alloca (tdep->size_ulong); > + > + regcache_raw_read_unsigned (regcache, tdep->arg2, > + &tmpulongest); > + if (tmpulongest) > + { > + tmpulongest += tdep->size_ulong; > + if (target_read_memory ((CORE_ADDR) tmpulongest, a, > + tdep->size_ulong)) > + { > + if (record_debug) > + fprintf_unfiltered (gdb_stdlog, > + "Process record: error reading " > + "memory at addr = 0x%s len = %d.\n", > + OUTPUT_REG (tmpulongest, tdep->arg2), > + tdep->size_ulong); > + return -1; > + } > + tmpulongest = extract_unsigned_integer (a, tdep->size_ulong, > + byte_order); > + if (record_linux_msghdr (regcache, tdep, > + extract_unsigned_integer > + (a, tdep->size_ulong, byte_order))) And here? > + case 528: > + /* sys_shmdt */ > + /* XXX maybe need do some record works wiht sys_shmdt. */ "with". > + case RECORD_SEMOP: > + case RECORD_SEMGET: > + case RECORD_SEMTIMEDOP: > + case RECORD_MSGSND: > + case RECORD_MSGGET: > + /* XXX maybe need do some record works wiht RECORD_SHMDT. */ "with". That's all I've got! After those, I'm good. Mark? How about you? Michael