Re: [PATCH] Implement new `info core mappings' command

[Eli Zaretskii <eliz@gnu.org>]

> From: Sergio Durigan Junior <sergiodj@redhat.com>
> Date: Wed, 26 Oct 2011 19:07:45 -0200
> 
>  *** Changes since GDB 7.3.1
>  
> +* GDB has a new `info core mappings' command.  It displays the memory
> +  regions in a corefile, similar to `info proc mappings' command.

This is okay.

> +  while (argv != NULL && *argv != NULL)
> +    {
> +      if (strncmp (argv[0], "mappings", strlen (argv[0])) == 0)
> +	{
> +	  mappings_f = 1;
> +	}
> +      else if (strncmp (argv[0], "all", strlen (argv[0])) == 0)
> +	{
> +	  all = 1;
> +	}
> +      argv++;
> +    }

What is this "all" stuff about?

> +@cindex core dump file

This index entry is too general, it sounds like this section describes
everything about core dump files that GDB supports.  Better qualify
it, e.g.

  @cindex core dump file, list mapped memory

> +@cindex memory address space mappings

Likewise; you need to qualify this so it's clear it only talks about
memory mappings in the context of core file debugging.

> +@item info core
> +@item info core mappings

You cannot have 2 @item's in a row.  All but the first one need to be
@itemx.


> +Report the memory address space ranges accessible in the core file.

I would lose the "space" part.

I think a short example of output would be good here.

> One thing I am not sure is where to put the entry for this command on
> the documentation.  I decided to put it below `info proc', but I'd be
> glad if you could give your opinions.

I don't think it's good to put it in the same section as "info proc",
because we say at the beginning of the section

  Many versions of SVR4 and compatible systems provide a facility called
  @samp{/proc} that can be used to examine the image of a running
  process using file-system subroutines.

But this new command has nothing to do with /proc, and does not need
/proc support to work, right?

If /proc is indeed irrelevant, then I'd prefer a separate @subsection
alongside this one.  You'd need to add some short explanation of the
background and use case(s) for this command, but having that is a good
idea anyway: as written now, this command lands on the reader out of
the blue, more or less.

Thanks.