Re: [PATCH 2/4] [nto] Fixes for nto procfs.

Pedro Alves
2015-10-15T17:41:19Z

It would have been nicer to see this split into a fix/theme
per patch, and add something to the commit log about each
fix. E.g., the aux bits could easily be a separate patch.

Anyway, this is pretty isolated to NTO bits.

LGTM with the nits below addressed.

On 10/13/2015 05:01 PM, Aleksandar Ristovski wrote:

> }
>
> do_cleanups (inner_cleanup);
> @@ -599,9 +612,40 @@ procfs_files_info (struct target_ops *ignore)
>
> printf_unfiltered ("\tUsing the running image of %s %s via %s.\n",
> inf->attach_flag ? "attached" : "child",
> - target_pid_to_str (inferior_ptid), nto_procfs_path);
> + target_pid_to_str (inferior_ptid),
> + nodestr ? nodestr : "local node");

Write 'nodestr != NULL'.

> +}
> +
> +/* Read executable file name for the given PID. */
> +
> +static char *
> +procfs_pid_to_exec_file (struct target_ops *ops, const int pid)
> +{
> + int proc_fd;
> + static char proc_path[PATH_MAX];
> + ssize_t rd;
> +
> + /* Read exe file name. */
> + snprintf (proc_path, sizeof (proc_path), "%s/proc/%d/exefile",
> + nodestr ? nodestr : "", pid);

Ditto.

> + proc_fd = open (proc_path, O_RDONLY);
> + if (proc_fd == -1)
> + return NULL;
> +
> + rd = read (proc_fd, proc_path, sizeof (proc_path) - 1);
> + close (proc_fd);
> + if (rd <= 0)
> + {
> + proc_path[0] = '\0';
> + return NULL;
> + }
> + else
> + proc_path[rd] = '\0';
> +
> + return proc_path;

Either write:

else
{
proc_path[rd] = '\0';
return proc_path;
}

Or drop the "else".

> }
>
> +
> /* Attach to process PID, then initialize for debugging it. */
> static void
> procfs_attach (struct target_ops *ops, const char *args, int from_tty)
> @@ -653,8 +697,8 @@ do_attach (ptid_t ptid)
> struct sigevent event;
> char path[PATH_MAX];
>
> - snprintf (path, PATH_MAX - 1, "%s/%d/as", nto_procfs_path,
> - ptid_get_pid (ptid));
> + snprintf (path, PATH_MAX - 1, "%s%s/%d/as", nodestr ? nodestr : "",
> + "/proc", ptid_get_pid (ptid));

nodestr != NULL

> ctl_fd = open (path, O_RDWR);
> if (ctl_fd == -1)
> error (_("Couldn't open proc file %s, error %d (%s)"), path, errno,
> @@ -872,6 +916,40 @@ procfs_xfer_partial (struct target_ops *ops, enum target_object object,
> {
> case TARGET_OBJECT_MEMORY:
> return procfs_xfer_memory (readbuf, writebuf, offset, len, xfered_len);
> + case TARGET_OBJECT_AUXV:
> + if (readbuf != NULL)
> + {
> + int err;
> + CORE_ADDR initial_stack;
> + debug_process_t procinfo;
> + /* For 32-bit architecture, size of auxv_t is 8 bytes. */
> + const unsigned int sizeof_auxv_t = sizeof (auxv_t);
> + const unsigned int sizeof_tempbuf = 20 * sizeof_auxv_t;
> + int tempread;
> + gdb_byte *const tempbuf = alloca (sizeof_tempbuf);
> +
> + if (!tempbuf)
> + return TARGET_XFER_E_IO;

if (tempbuf == NULL)

Can NTO's alloca really return NULL?

> +
> + err = devctl (ctl_fd, DCMD_PROC_INFO, &procinfo,
> + sizeof procinfo, 0);
> + if (err != EOK)
> + return TARGET_XFER_E_IO;

Thanks,
Pedro Alves