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

[Pedro Alves <palves@redhat.com>]

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