This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 1/2][PR gdb/20239] Make evaluation and type-printing of all NonZero-optimized enums work
- From: Tom Tromey <tom at tromey dot com>
- To: Manish Goregaokar <manish at mozilla dot com>
- Cc: gdb-patches at sourceware dot org, Tom Tromey <tom at tromey dot com>
- Date: Wed, 22 Jun 2016 13:10:11 -0600
- Subject: Re: [PATCH 1/2][PR gdb/20239] Make evaluation and type-printing of all NonZero-optimized enums work
- Authentication-results: sourceware.org; auth=none
- References: <CAFOnWknif9WwaRdDYzxx8ngNJBwgfN24w02KGPVnYA6-VtMcUg at mail dot gmail dot com>
>>>>> "Manish" == Manish Goregaokar <manish@mozilla.com> writes:
Manish> Regarding the xstrdup, this is done because strsep edits the string
Manish> itself.
Indeed it does. Somehow I didn't realize this, sorry about that.
Manish> Methods tests fail, but they are
Manish> a preexisting failure due to Rust's debuginfo changing.
Yes, the situation with 1.9 is not that good :(
This patch looks good. There are formatting nits but really nothing serious.
The normal procedure is that once you've written one good patch you can
get write-after-approval access to gdb. If you want to go this route,
let me know. Otherwise (and also let me know) I can push the final
patches for you.
Manish> + char *type_name;
I think this is assigned to but not used, so you might as well remove
it.
Manish> + /* Optimized enums have only one field */
gdb style is period with two spaces at the end of a comment.
Manish> + while ((token = strsep (&tail, "$")) != NULL)
Manish> + {
The "{" should be indented 2 more spaces, and then the body as well.
Manish> + if (sscanf (token, "%lu", &fieldno) != 1)
Manish> + {
Here too.
Manish> + /* We have reached the enum name, which cannot start with a digit */
The period thing again. Also this line might be too long.
Manish> + };
Extra ";".
Manish> + value = unpack_long (member_type,
Manish> + valaddr + embedded_offset);
Make sure the continuation line is indented to line up under "member_type".
(Maybe it is but I can't tell ...)
Manish> int i, len = 0;
Manish> + int skip_to = 1; /* Skip the discriminant field */
Indentation? Also gdb doesn't tend to use trailing comments like that;
you can put it before the declaration.
Manish> + char *zero_field = strrchr (TYPE_FIELD_NAME (type, 0), '$');
const char *
Manish> + if (zero_field != NULL && strlen (zero_field) > 1) {
"{" on the next line and indented.
Manish> + fprintfi_filtered (level+2, stream, "%s,\n", zero_field+1);
Spaces around "+"s.
Manish> + /* there is no explicit discriminant field, skip nothing */
Upper case "There", period.
Manish> - int first = 1;
Manish> + int first = 1;
Some spurious change maybe?
Tom