This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: [PATCH] Add support for tracking/evaluating dwarf2 location expressions


Dan, just some things to tweek,

State machines are normally implemented using a switch and not a chain
of ifs.  Check dwarf2read.c:decode_locdesc() as an example (hmm, slight
dejavu :-).

While fall through switches such as:

+         /* Determine the size to read, and whether it's signed or
+            not */
+         switch (op)
+           {
+           case DW_OP_const1s:
+             sign = 1;
+           case DW_OP_const1u:
+             readsize = 1;
+             break;

might be cute they tend to make the life of those that follow painful. 
As they say, dumb the code down.

Don't forget ``a = b'' not ``a=b''.

Just use ``struct value *'', I've every intention of zapping
``value_ptr''! :-)

  value_ptr stack[64];
Is there a constant for this?  A quick glance at decode_locdesc() and it
has the same hardwired constant.

>From memory, the dwarf2 state machine doco states that you should return
the top-of-stack when the machine has finished executing.  This means
that the stack may not be empty and that the code could potentially leak
``struct value *''s.

	enjoy,
		Andrew


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]