Re: [patch 1/2] build id

[Eli Zaretskii <eliz@gnu.org>]

> Date: Tue, 23 Nov 2010 11:27:36 -0500
> From: sami wagiaalla <swagiaal@redhat.com>
> 
> This patch comes from the Fedora rpm and supports the use of build id 
> for locating and loading core file executables, and printing information 
> about missing debug info files.

Thanks.

> +  add_setshow_boolean_cmd ("build-id-core-loads", class_files,
> +			   &build_id_core_loads, _("\
> +Set whether CORE-FILE loads the build-id associated files automatically."), _("\
> +Show whether CORE-FILE loads the build-id associated files automatically.."),

If by CORE-FILE you mean the command "core-file", then I suggest to
use \"core-file\" here.

> +You can also adjust the current verbosity of the @dfn{build id} locating.

The @dfn markup should be used only once, when the terminology is
first used and explained.  Thereafter, you should not use @dfn, but
rather regular text.

> +@kindex set build-id-verbose
> +@item set build-id-verbose 0
> +No additional messages are printed.

It is better to say  "Verbose messages are off."

> +@item set build-id-verbose 1
> +Missing separate debug filenames are printed.

It's better to avoid passive tense:

  Print names of separate debug files that couldn't be found.

> +@item set build-id-verbose 2
> +Missing separate debug filenames are printed and also all the parsing of the
> +binaries to find their @dfn{build id} content is printed.

  Print missing separate debug files, and also be verbose about
  parsing of the binaries to find their build IDs.

> +#define BUILD_ID_VERBOSE_NONE 0
> +#define BUILD_ID_VERBOSE_FILENAMES 1
> +#define BUILD_ID_VERBOSE_BINARY_PARSE 2

Isn't it better to use an enum?

> +      /* FIXME: bad alignment assumption.  */

Should this FIXME be fixed before committing?

> +	  gdb_byte *data = (void *) descdata;

Why do you need this cast?