Re: [RFA 4/8] New port: TI C6x: Read loadmap from gdbserver

Mike Frysinger
2011-07-22T02:17:34Z

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