From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26823 invoked by alias); 22 Jul 2011 02:17:34 -0000 Received: (qmail 26812 invoked by uid 22791); 22 Jul 2011 02:17:33 -0000 X-SWARE-Spam-Status: No, hits=-2.5 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW X-Spam-Check-By: sourceware.org Received: from mail-iy0-f169.google.com (HELO mail-iy0-f169.google.com) (209.85.210.169) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 22 Jul 2011 02:17:17 +0000 Received: by iyb14 with SMTP id 14so1769226iyb.0 for ; Thu, 21 Jul 2011 19:17:16 -0700 (PDT) Received: by 10.231.28.17 with SMTP id k17mr832058ibc.99.1311301036459; Thu, 21 Jul 2011 19:17:16 -0700 (PDT) MIME-Version: 1.0 Received: by 10.42.217.71 with HTTP; Thu, 21 Jul 2011 19:16:56 -0700 (PDT) In-Reply-To: <4E263865.2070100@codesourcery.com> References: <4E263865.2070100@codesourcery.com> From: Mike Frysinger Date: Fri, 22 Jul 2011 12:22:00 -0000 Message-ID: Subject: Re: [RFA 4/8] New port: TI C6x: Read loadmap from gdbserver To: Yao Qi Cc: gdb-patches@sourceware.org Content-Type: text/plain; charset=ISO-8859-1 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-07/txt/msg00605.txt.bz2 On Tue, Jul 19, 2011 at 22:07, Yao Qi wrote: > +#if defined __DSBT__ > +static int rather than being tied to the exec format that *gdbserver* is being built as, shouldnt this be bound to the ptrace defines being available ? how abut using "#ifdef PTRACE_GETDSBT" ? > +linux_read_loadmap (const char *annex, CORE_ADDR offset, > + unsigned char *myaddr, unsigned int len) indentation looks wrong wrt GNU style. needs tabs and spaces to properly align it. > + int addr = (strcmp (annex, "exec") == 0 ? (int) PTRACE_GETDSBT_EXEC : > + (strcmp (annex, "interp") == 0 ? (int) PTRACE_GETDSBT_INTERP : > + -1)); > ... > + if (addr == -1) > + return -1; style with add init is off here too. but seems like it'd be cleaner to have this be a series of if statements incorporated into the -1 check to make the -1 value unnecessary. > + copy_length = actual_length - offset < len ? actual_length - offset : len; does gdb really not have min/max helpers ? > + ptrace (PTRACE_GETDSBT, pid, addr, &data); what if it fails ? > + {"fdpic", handle_qxfer_fdpic} style is broken. needs space after "{", space before "}", and comma after "}" > + /* Load maps for FDPIC systems. */ > + TARGET_OBJECT_FDPIC i think this needs a trailing comma -mike