Re: [patch 1/2] build id

Eli Zaretskii
2010-11-23T19:10:11Z

> 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?