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

[Mike Frysinger <vapier at gentoo dot org>]

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