This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: [RFA] New bitflags type and eflags on i386/x86-64
- From: Andrew Cagney <ac131313 at ges dot redhat dot com>
- To: Mark Kettenis <kettenis at chello dot nl>
- Cc: mludvig at suse dot cz, drow at mvista dot com, gdb-patches at sources dot redhat dot com,ac131313 at redhat dot com
- Date: Mon, 09 Sep 2002 23:35:19 -0400
- Subject: Re: [RFA] New bitflags type and eflags on i386/x86-64
- References: <3D6BF1D5.70409@ges.redhat.com> <3D6E231D.8060906@suse.cz> <20020829142120.GA5176@nevyn.them.org> <3D6E3666.7070309@suse.cz> <20020829150833.GA29973@nevyn.them.org> <3D6E40EE.5000904@ges.redhat.com> <4.2.0.58.20020830155945.00a473b0@ics.u-strasbg.fr> <3D6F81F5.2040002@suse.cz> <4.2.0.58.20020830164921.01c6fba8@ics.u-strasbg.fr> <3D747E37.7060005@suse.cz> <20020903123025.GA22555@nevyn.them.org> <3D770FFB.5030802@suse.cz> <200209061959.g86JxhK0000467@elgar.kettenis.dyndns.org>
Date: Thu, 05 Sep 2002 10:04:11 +0200
From: Michal Ludvig <mludvig@suse.cz>
Daniel Jacobowitz wrote:
> On Tue, Sep 03, 2002 at 11:17:43AM +0200, Michal Ludvig wrote:
>>Thanks for help. Attached file incorporates your patch as well. Could
>>anyone approve it, please?
>
> Can't approve it, but I like it. Thanks for following this through.
Who can approve? Andrew? MarkK? Someone else?
Well, I'm not too happy about the introduction of i386-common-tdep.c.
I mean, those functions belong in i386-tdep.c. Can we postpone
integrating this patch until I've unified the i386 and x86-64 targets
into one truly multi-arched one? I've started working on that now,
honestly :-).
Anyway, before approving this I'd like to see a bit more consensus
about the BitFlags type your patch introduces. Andrew, is it OK with
you now?
Daniel was the one with concerns, I believe they were addressed.
Michael, what does a user interaction where values are set/cleared look
like? It should be pretty obvious without needing to look at a manual.
Andrew
There also some coding-style problems with your code:
Index: gdbtypes.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbtypes.c,v
retrieving revision 1.56
diff -u -p -r1.56 gdbtypes.c
--- gdbtypes.c 20 Aug 2002 19:57:32 -0000 1.56
+++ gdbtypes.c 30 Aug 2002 13:42:50 -0000
@@ -782,6 +782,61 @@ create_set_type (struct type *result_typ
return (result_type);
}
+/*
+ * - The following three functions are intended to be used for BitFlags
+ * types (e.g. i386's EFLAGS register).
+ * - A BitFlags type is an integer where bits may have a symbolic name
+ * to be printed when the bit is set.
+ * - Printing is done in <lang>_val_print() under a TYPE_CODE_FLAGS label.
+ * - Add symbolic names for relevant bits using add_flag_name() after
+ * initializing the BitFlags type.
+ */
That's not the right comment style.
+void
+add_flag_ignore (struct type *type, int bitpos)
+{
+ TYPE_FIELD_BITPOS (type, bitpos) = -1;
+}
Indentation is wrong here...
+void
+add_flag_name (struct type *type, int bitpos, char *name)
+{
+ int namelen;
+
+ gdb_assert (TYPE_CODE (type) == TYPE_CODE_FLAGS);
+ gdb_assert (bitpos < TYPE_NFIELDS (type));
+ gdb_assert (bitpos >= 0);
+
+ namelen=strlen(name)+1;
+ TYPE_FIELD_NAME (type, bitpos) = xmalloc (namelen);
+ snprintf(TYPE_FIELD_NAME (type, bitpos), namelen, "%s", name);
+
+ TYPE_FIELD_BITPOS (type, bitpos) = bitpos;
+}
...and here.
+struct type *
+init_flags_type (int bitlength, char *name, struct objfile *objfile)
+{
+ register struct type *type;
We're trying to get rid of "register".
Index: gdbtypes.h
===================================================================
RCS file: /cvs/src/src/gdb/gdbtypes.h,v
retrieving revision 1.35
diff -u -p -r1.35 gdbtypes.h
--- gdbtypes.h 10 Aug 2002 05:12:40 -0000 1.35
+++ gdbtypes.h 30 Aug 2002 13:42:51 -0000
@@ -1054,6 +1056,14 @@ extern struct type *alloc_type (struct o
extern struct type *init_type (enum type_code, int, int, char *,
struct objfile *);
+
+/* Helper functions to construct BitField type.
+ See description in gdbarch.c for details. */
Are you sure this reference to gdbarch.c is still correct?
Mark