This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA 4/8] New port: TI C6x: Read loadmap from gdbserver
- From: Mike Frysinger <vapier at gentoo dot org>
- To: Yao Qi <yao at codesourcery dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Thu, 21 Jul 2011 22:16:56 -0400
- Subject: Re: [RFA 4/8] New port: TI C6x: Read loadmap from gdbserver
- References: <4E263865.2070100@codesourcery.com>
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