[PATCH] Introduce and use disassembly_flags

[PATCH] Introduce and use disassembly_flags

[Simon Marchi]

This patch defines a new enum flags type, disassembly_flags, and uses
it.  Nothing fancy.

gdb/ChangeLog:

	* disasm.h (disassembly_flag): New enum.
	(DISASSEMBLY_SOURCE_DEPRECATED, DISASSEMBLY_RAW_INSN,
	DISASSEMBLY_OMIT_FNAME, DISASSEMBLY_FILENAME,
	DISASSEMBLY_OMIT_PC, DISASSEMBLY_SOURCE,
	DISASSEMBLY_SPECULATIVE): Move to enum disassembly_flag.
	(disassembly_flags): New enum_flags type.
	(gdb_disassembly, gdb_pretty_print_disassembler::pretty_print_insn):
	Use disassembly_flags.
	* disasm.c (gdb_pretty_print_disassembler::pretty_print_insn,
	dump_insns, do_mixed_source_and_assembly_deprecated,
	do_mixed_source_and_assembly, do_assembly_only,
	gdb_disassembly): Likewise.
	* record-btrace.c (btrace_print_lines, btrace_insn_history,
	record_btrace_insn_history, record_btrace_insn_history_range,
	record_btrace_insn_history_from): Likewise.
	* record.c (get_insn_history_modifiers,
	cmd_record_insn_history): Likewise.
	* target-debug.h (target_debug_print_disassembly_flags): New
	macro.
	* target-delegates.c: Re-generate.
	* target.c (target_insn_history, target_insn_history_from,
	target_insn_history_range): Use disassembly_flags.
	* target.h (struct target_ops) <to_insn_history,
	to_insn_history_from, to_insn_history_range>: Likewise.
	(target_insn_history, target_insn_history_from,
	target_insn_history_range): Likewise.
	* cli/cli-cmds.c (print_disassembly,
	disassemble_current_function, disassemble_command): Likewise.
	* mi/mi-cmd-disas.c (mi_cmd_disassemble): Likewise.
---
 gdb/cli/cli-cmds.c     |  7 +++----
 gdb/disasm.c           | 12 ++++++------
 gdb/disasm.h           | 22 +++++++++++++---------
 gdb/mi/mi-cmd-disas.c  |  4 ++--
 gdb/record-btrace.c    | 26 ++++++++++++++++----------
 gdb/record.c           |  7 ++++---
 gdb/target-debug.h     |  2 ++
 gdb/target-delegates.c | 24 ++++++++++++------------
 gdb/target.c           |  7 ++++---
 gdb/target.h           | 18 ++++++++++++------
 10 files changed, 74 insertions(+), 55 deletions(-)

diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index b5c9d0ba78..5f33105e23 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -1134,7 +1134,7 @@ list_command (char *arg, int from_tty)
 
 static void
 print_disassembly (struct gdbarch *gdbarch, const char *name,
-		   CORE_ADDR low, CORE_ADDR high, int flags)
+		   CORE_ADDR low, CORE_ADDR high, disassembly_flags flags)
 {
 #if defined(TUI)
   if (!tui_is_window_visible (DISASSEM_WIN))
@@ -1165,7 +1165,7 @@ print_disassembly (struct gdbarch *gdbarch, const char *name,
    Print a disassembly of the current function according to FLAGS.  */
 
 static void
-disassemble_current_function (int flags)
+disassemble_current_function (disassembly_flags flags)
 {
   struct frame_info *frame;
   struct gdbarch *gdbarch;
@@ -1220,12 +1220,11 @@ disassemble_command (char *arg, int from_tty)
   CORE_ADDR low, high;
   const char *name;
   CORE_ADDR pc;
-  int flags;
+  disassembly_flags flags;
   const char *p;
 
   p = arg;
   name = NULL;
-  flags = 0;
 
   if (p && *p == '/')
     {
diff --git a/gdb/disasm.c b/gdb/disasm.c
index 64d6684b53..4a3bfdd6fc 100644
--- a/gdb/disasm.c
+++ b/gdb/disasm.c
@@ -184,7 +184,7 @@ compare_lines (const void *mle1p, const void *mle2p)
 int
 gdb_pretty_print_disassembler::pretty_print_insn (struct ui_out *uiout,
 						  const struct disasm_insn *insn,
-						  int flags)
+						  disassembly_flags flags)
 {
   /* parts of the symbolic representation of the address */
   int unmapped;
@@ -288,7 +288,7 @@ gdb_pretty_print_disassembler::pretty_print_insn (struct ui_out *uiout,
 static int
 dump_insns (struct gdbarch *gdbarch,
 	    struct ui_out *uiout, CORE_ADDR low, CORE_ADDR high,
-	    int how_many, int flags, CORE_ADDR *end_pc)
+	    int how_many, disassembly_flags flags, CORE_ADDR *end_pc)
 {
   struct disasm_insn insn;
   int num_displayed = 0;
@@ -331,7 +331,7 @@ do_mixed_source_and_assembly_deprecated
   (struct gdbarch *gdbarch, struct ui_out *uiout,
    struct symtab *symtab,
    CORE_ADDR low, CORE_ADDR high,
-   int how_many, int flags)
+   int how_many, disassembly_flags flags)
 {
   int newlines = 0;
   int nlines;
@@ -494,7 +494,7 @@ do_mixed_source_and_assembly (struct gdbarch *gdbarch,
 			      struct ui_out *uiout,
 			      struct symtab *main_symtab,
 			      CORE_ADDR low, CORE_ADDR high,
-			      int how_many, int flags)
+			      int how_many, disassembly_flags flags)
 {
   const struct linetable_entry *le, *first_le;
   int i, nlines;
@@ -730,7 +730,7 @@ do_mixed_source_and_assembly (struct gdbarch *gdbarch,
 static void
 do_assembly_only (struct gdbarch *gdbarch, struct ui_out *uiout,
 		  CORE_ADDR low, CORE_ADDR high,
-		  int how_many, int flags)
+		  int how_many, disassembly_flags flags)
 {
   struct cleanup *ui_out_chain;
 
@@ -806,7 +806,7 @@ gdb_disassembler::print_insn (CORE_ADDR memaddr,
 
 void
 gdb_disassembly (struct gdbarch *gdbarch, struct ui_out *uiout,
-		 int flags, int how_many,
+		 disassembly_flags flags, int how_many,
 		 CORE_ADDR low, CORE_ADDR high)
 {
   struct symtab *symtab;
diff --git a/gdb/disasm.h b/gdb/disasm.h
index 42c1f3af6d..bb7a7a0d5c 100644
--- a/gdb/disasm.h
+++ b/gdb/disasm.h
@@ -21,13 +21,17 @@
 
 #include "dis-asm.h"
 
-#define DISASSEMBLY_SOURCE_DEPRECATED (0x1 << 0)
-#define DISASSEMBLY_RAW_INSN	(0x1 << 1)
-#define DISASSEMBLY_OMIT_FNAME	(0x1 << 2)
-#define DISASSEMBLY_FILENAME	(0x1 << 3)
-#define DISASSEMBLY_OMIT_PC	(0x1 << 4)
-#define DISASSEMBLY_SOURCE	(0x1 << 5)
-#define DISASSEMBLY_SPECULATIVE	(0x1 << 6)
+enum disassembly_flag {
+  DISASSEMBLY_SOURCE_DEPRECATED = (1 << 0),
+  DISASSEMBLY_RAW_INSN = (1 << 1),
+  DISASSEMBLY_OMIT_FNAME = (1 << 2),
+  DISASSEMBLY_FILENAME = (1 << 3),
+  DISASSEMBLY_OMIT_PC = (1 << 4),
+  DISASSEMBLY_SOURCE = (1 << 5),
+  DISASSEMBLY_SPECULATIVE = (1 << 6),
+};
+
+DEF_ENUM_FLAGS_TYPE (disassembly_flag, disassembly_flags);
 
 struct gdbarch;
 struct ui_out;
@@ -87,7 +91,7 @@ struct disasm_insn
 };
 
 extern void gdb_disassembly (struct gdbarch *gdbarch, struct ui_out *uiout,
-			     int flags, int how_many,
+			     disassembly_flags flags, int how_many,
 			     CORE_ADDR low, CORE_ADDR high);
 
 /* Print the instruction at address MEMADDR in debugged memory,
@@ -109,7 +113,7 @@ public:
   /* Prints the instruction INSN into UIOUT and returns the length of
      the printed instruction in bytes.  */
   int pretty_print_insn (struct ui_out *uiout, const struct disasm_insn *insn,
-			 int flags);
+			 disassembly_flags flags);
 
 private:
   /* Returns the architecture used for disassembling.  */
diff --git a/gdb/mi/mi-cmd-disas.c b/gdb/mi/mi-cmd-disas.c
index 9338f7bdec..d4023335b5 100644
--- a/gdb/mi/mi-cmd-disas.c
+++ b/gdb/mi/mi-cmd-disas.c
@@ -57,7 +57,8 @@ mi_cmd_disassemble (char *command, char **argv, int argc)
   struct ui_out *uiout = current_uiout;
   CORE_ADDR start;
 
-  int mode, disasm_flags;
+  int mode;
+  disassembly_flags disasm_flags;
   struct symtab *s;
 
   /* Which options have we processed ... */
@@ -147,7 +148,6 @@ mi_cmd_disassemble (char *command, char **argv, int argc)
 
   /* Convert the mode into a set of disassembly flags.  */
 
-  disasm_flags = 0;  /* Initialize here for -Wall.  */
   switch (mode)
     {
     case 0:
diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index daa06968a0..5465692df5 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -669,7 +669,7 @@ btrace_find_line_range (CORE_ADDR pc)
 
 static void
 btrace_print_lines (struct btrace_line_range lines, struct ui_out *uiout,
-		    struct cleanup **ui_item_chain, int flags)
+		    struct cleanup **ui_item_chain, disassembly_flags flags)
 {
   print_source_lines_flags psl_flags;
   int line;
@@ -698,14 +698,16 @@ static void
 btrace_insn_history (struct ui_out *uiout,
 		     const struct btrace_thread_info *btinfo,
 		     const struct btrace_insn_iterator *begin,
-		     const struct btrace_insn_iterator *end, int flags)
+		     const struct btrace_insn_iterator *end,
+		     disassembly_flags flags)
 {
   struct cleanup *cleanups, *ui_item_chain;
   struct gdbarch *gdbarch;
   struct btrace_insn_iterator it;
   struct btrace_line_range last_lines;
 
-  DEBUG ("itrace (0x%x): [%u; %u)", flags, btrace_insn_number (begin),
+  DEBUG ("itrace (%s): [%u; %u)", hex_string (flags),
+	 btrace_insn_number (begin),
 	 btrace_insn_number (end));
 
   flags |= DISASSEMBLY_SPECULATIVE;
@@ -784,7 +786,8 @@ btrace_insn_history (struct ui_out *uiout,
 /* The to_insn_history method of target record-btrace.  */
 
 static void
-record_btrace_insn_history (struct target_ops *self, int size, int flags)
+record_btrace_insn_history (struct target_ops *self, int size,
+			    disassembly_flags flags)
 {
   struct btrace_thread_info *btinfo;
   struct btrace_insn_history *history;
@@ -806,7 +809,7 @@ record_btrace_insn_history (struct target_ops *self, int size, int flags)
     {
       struct btrace_insn_iterator *replay;
 
-      DEBUG ("insn-history (0x%x): %d", flags, size);
+      DEBUG ("insn-history (%s): %d", hex_string (flags), size);
 
       /* If we're replaying, we start at the replay position.  Otherwise, we
 	 start at the tail of the trace.  */
@@ -838,8 +841,9 @@ record_btrace_insn_history (struct target_ops *self, int size, int flags)
       begin = history->begin;
       end = history->end;
 
-      DEBUG ("insn-history (0x%x): %d, prev: [%u; %u)", flags, size,
-	     btrace_insn_number (&begin), btrace_insn_number (&end));
+      DEBUG ("insn-history (%s): %d, prev: [%u; %u)",
+	     hex_string (flags), size, btrace_insn_number (&begin),
+	     btrace_insn_number (&end));
 
       if (size < 0)
 	{
@@ -871,7 +875,8 @@ record_btrace_insn_history (struct target_ops *self, int size, int flags)
 
 static void
 record_btrace_insn_history_range (struct target_ops *self,
-				  ULONGEST from, ULONGEST to, int flags)
+				  ULONGEST from, ULONGEST to,
+				  disassembly_flags flags)
 {
   struct btrace_thread_info *btinfo;
   struct btrace_insn_history *history;
@@ -887,7 +892,7 @@ record_btrace_insn_history_range (struct target_ops *self,
   low = from;
   high = to;
 
-  DEBUG ("insn-history (0x%x): [%u; %u)", flags, low, high);
+  DEBUG ("insn-history (%s): [%u; %u)", hex_string (flags), low, high);
 
   /* Check for wrap-arounds.  */
   if (low != from || high != to)
@@ -924,7 +929,8 @@ record_btrace_insn_history_range (struct target_ops *self,
 
 static void
 record_btrace_insn_history_from (struct target_ops *self,
-				 ULONGEST from, int size, int flags)
+				 ULONGEST from, int size,
+				 disassembly_flags flags)
 {
   ULONGEST begin, end, context;
 
diff --git a/gdb/record.c b/gdb/record.c
index 15271b2667..e720c11bbf 100644
--- a/gdb/record.c
+++ b/gdb/record.c
@@ -428,10 +428,10 @@ no_chunk (char *arg)
 
 /* Read instruction-history modifiers from an argument string.  */
 
-static int
+static disassembly_flags
 get_insn_history_modifiers (char **arg)
 {
-  int modifiers;
+  disassembly_flags modifiers;
   char *args;
 
   modifiers = 0;
@@ -509,7 +509,8 @@ command_size_to_target_size (unsigned int size)
 static void
 cmd_record_insn_history (char *arg, int from_tty)
 {
-  int flags, size;
+  disassembly_flags flags;
+  int size;
 
   require_record_target ();
 
diff --git a/gdb/target-debug.h b/gdb/target-debug.h
index 857ceceae6..73e68d4cea 100644
--- a/gdb/target-debug.h
+++ b/gdb/target-debug.h
@@ -160,6 +160,8 @@
   target_debug_do_print (host_address_to_string (X))
 #define target_debug_print_enum_remove_bp_reason(X) \
   target_debug_do_print (plongest (X))
+#define target_debug_print_disassembly_flags(X) \
+  target_debug_do_print (phex (X, 0))
 
 static void
 target_debug_print_struct_target_waitstatus_p (struct target_waitstatus *status)
diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
index 73e45dd8a7..1bb813c3ae 100644
--- a/gdb/target-delegates.c
+++ b/gdb/target-delegates.c
@@ -3825,20 +3825,20 @@ debug_goto_record (struct target_ops *self, ULONGEST arg1)
 }
 
 static void
-delegate_insn_history (struct target_ops *self, int arg1, int arg2)
+delegate_insn_history (struct target_ops *self, int arg1, disassembly_flags arg2)
 {
   self = self->beneath;
   self->to_insn_history (self, arg1, arg2);
 }
 
 static void
-tdefault_insn_history (struct target_ops *self, int arg1, int arg2)
+tdefault_insn_history (struct target_ops *self, int arg1, disassembly_flags arg2)
 {
   tcomplain ();
 }
 
 static void
-debug_insn_history (struct target_ops *self, int arg1, int arg2)
+debug_insn_history (struct target_ops *self, int arg1, disassembly_flags arg2)
 {
   fprintf_unfiltered (gdb_stdlog, "-> %s->to_insn_history (...)\n", debug_target.to_shortname);
   debug_target.to_insn_history (&debug_target, arg1, arg2);
@@ -3847,25 +3847,25 @@ debug_insn_history (struct target_ops *self, int arg1, int arg2)
   fputs_unfiltered (", ", gdb_stdlog);
   target_debug_print_int (arg1);
   fputs_unfiltered (", ", gdb_stdlog);
-  target_debug_print_int (arg2);
+  target_debug_print_disassembly_flags (arg2);
   fputs_unfiltered (")\n", gdb_stdlog);
 }
 
 static void
-delegate_insn_history_from (struct target_ops *self, ULONGEST arg1, int arg2, int arg3)
+delegate_insn_history_from (struct target_ops *self, ULONGEST arg1, int arg2, disassembly_flags arg3)
 {
   self = self->beneath;
   self->to_insn_history_from (self, arg1, arg2, arg3);
 }
 
 static void
-tdefault_insn_history_from (struct target_ops *self, ULONGEST arg1, int arg2, int arg3)
+tdefault_insn_history_from (struct target_ops *self, ULONGEST arg1, int arg2, disassembly_flags arg3)
 {
   tcomplain ();
 }
 
 static void
-debug_insn_history_from (struct target_ops *self, ULONGEST arg1, int arg2, int arg3)
+debug_insn_history_from (struct target_ops *self, ULONGEST arg1, int arg2, disassembly_flags arg3)
 {
   fprintf_unfiltered (gdb_stdlog, "-> %s->to_insn_history_from (...)\n", debug_target.to_shortname);
   debug_target.to_insn_history_from (&debug_target, arg1, arg2, arg3);
@@ -3876,25 +3876,25 @@ debug_insn_history_from (struct target_ops *self, ULONGEST arg1, int arg2, int a
   fputs_unfiltered (", ", gdb_stdlog);
   target_debug_print_int (arg2);
   fputs_unfiltered (", ", gdb_stdlog);
-  target_debug_print_int (arg3);
+  target_debug_print_disassembly_flags (arg3);
   fputs_unfiltered (")\n", gdb_stdlog);
 }
 
 static void
-delegate_insn_history_range (struct target_ops *self, ULONGEST arg1, ULONGEST arg2, int arg3)
+delegate_insn_history_range (struct target_ops *self, ULONGEST arg1, ULONGEST arg2, disassembly_flags arg3)
 {
   self = self->beneath;
   self->to_insn_history_range (self, arg1, arg2, arg3);
 }
 
 static void
-tdefault_insn_history_range (struct target_ops *self, ULONGEST arg1, ULONGEST arg2, int arg3)
+tdefault_insn_history_range (struct target_ops *self, ULONGEST arg1, ULONGEST arg2, disassembly_flags arg3)
 {
   tcomplain ();
 }
 
 static void
-debug_insn_history_range (struct target_ops *self, ULONGEST arg1, ULONGEST arg2, int arg3)
+debug_insn_history_range (struct target_ops *self, ULONGEST arg1, ULONGEST arg2, disassembly_flags arg3)
 {
   fprintf_unfiltered (gdb_stdlog, "-> %s->to_insn_history_range (...)\n", debug_target.to_shortname);
   debug_target.to_insn_history_range (&debug_target, arg1, arg2, arg3);
@@ -3905,7 +3905,7 @@ debug_insn_history_range (struct target_ops *self, ULONGEST arg1, ULONGEST arg2,
   fputs_unfiltered (", ", gdb_stdlog);
   target_debug_print_ULONGEST (arg2);
   fputs_unfiltered (", ", gdb_stdlog);
-  target_debug_print_int (arg3);
+  target_debug_print_disassembly_flags (arg3);
   fputs_unfiltered (")\n", gdb_stdlog);
 }
 
diff --git a/gdb/target.c b/gdb/target.c
index 3c409f0f61..72f582ea20 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -3838,7 +3838,7 @@ target_goto_record (ULONGEST insn)
 /* See target.h.  */
 
 void
-target_insn_history (int size, int flags)
+target_insn_history (int size, disassembly_flags flags)
 {
   current_target.to_insn_history (&current_target, size, flags);
 }
@@ -3846,7 +3846,7 @@ target_insn_history (int size, int flags)
 /* See target.h.  */
 
 void
-target_insn_history_from (ULONGEST from, int size, int flags)
+target_insn_history_from (ULONGEST from, int size, disassembly_flags flags)
 {
   current_target.to_insn_history_from (&current_target, from, size, flags);
 }
@@ -3854,7 +3854,8 @@ target_insn_history_from (ULONGEST from, int size, int flags)
 /* See target.h.  */
 
 void
-target_insn_history_range (ULONGEST begin, ULONGEST end, int flags)
+target_insn_history_range (ULONGEST begin, ULONGEST end,
+			   disassembly_flags flags)
 {
   current_target.to_insn_history_range (&current_target, begin, end, flags);
 }
diff --git a/gdb/target.h b/gdb/target.h
index 8df117ece2..9b54195aa9 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -73,6 +73,7 @@ struct inferior;
 #include "gdb_signals.h"
 #include "btrace.h"
 #include "command.h"
+#include "disasm.h"
 
 #include "break-common.h" /* For enum target_hw_bp_type.  */
 
@@ -1195,7 +1196,8 @@ struct target_ops
        the current position.
        If SIZE < 0, disassemble abs (SIZE) preceding instructions; otherwise,
        disassemble SIZE succeeding instructions.  */
-    void (*to_insn_history) (struct target_ops *, int size, int flags)
+    void (*to_insn_history) (struct target_ops *, int size,
+			     disassembly_flags flags)
       TARGET_DEFAULT_NORETURN (tcomplain ());
 
     /* Disassemble SIZE instructions in the recorded execution trace around
@@ -1203,13 +1205,15 @@ struct target_ops
        If SIZE < 0, disassemble abs (SIZE) instructions before FROM; otherwise,
        disassemble SIZE instructions after FROM.  */
     void (*to_insn_history_from) (struct target_ops *,
-				  ULONGEST from, int size, int flags)
+				  ULONGEST from, int size,
+				  disassembly_flags flags)
       TARGET_DEFAULT_NORETURN (tcomplain ());
 
     /* Disassemble a section of the recorded execution trace from instruction
        BEGIN (inclusive) to instruction END (inclusive).  */
     void (*to_insn_history_range) (struct target_ops *,
-				   ULONGEST begin, ULONGEST end, int flags)
+				   ULONGEST begin, ULONGEST end,
+				   disassembly_flags flags)
       TARGET_DEFAULT_NORETURN (tcomplain ());
 
     /* Print a function trace of the recorded execution trace.
@@ -2514,13 +2518,15 @@ extern void target_goto_record_end (void);
 extern void target_goto_record (ULONGEST insn);
 
 /* See to_insn_history.  */
-extern void target_insn_history (int size, int flags);
+extern void target_insn_history (int size, disassembly_flags flags);
 
 /* See to_insn_history_from.  */
-extern void target_insn_history_from (ULONGEST from, int size, int flags);
+extern void target_insn_history_from (ULONGEST from, int size,
+				      disassembly_flags flags);
 
 /* See to_insn_history_range.  */
-extern void target_insn_history_range (ULONGEST begin, ULONGEST end, int flags);
+extern void target_insn_history_range (ULONGEST begin, ULONGEST end,
+				       disassembly_flags flags);
 
 /* See to_call_history.  */
 extern void target_call_history (int size, int flags);
-- 
2.11.0

Re: [PATCH] Introduce and use disassembly_flags

[Yao Qi]

On Sat, Feb 11, 2017 at 3:12 AM, Simon Marchi <simon.marchi@polymtl.ca> wrote:
> @@ -1220,12 +1220,11 @@ disassemble_command (char *arg, int from_tty)
>    CORE_ADDR low, high;
>    const char *name;
>    CORE_ADDR pc;
> -  int flags;
> +  disassembly_flags flags;
>    const char *p;
>
>    p = arg;
>    name = NULL;
> -  flags = 0;
>

We don't need to initialize 'flags' to zero?  The default construct of
enum_flags
doesn't nothing.

-- 
Yao (齐尧)

Re: [PATCH] Introduce and use disassembly_flags

[Luis Machado]

On 02/10/2017 09:12 PM, Simon Marchi wrote:
> diff --git a/gdb/disasm.h b/gdb/disasm.h
> index 42c1f3af6d..bb7a7a0d5c 100644
> --- a/gdb/disasm.h
> +++ b/gdb/disasm.h
> @@ -21,13 +21,17 @@
>
>  #include "dis-asm.h"
>
> -#define DISASSEMBLY_SOURCE_DEPRECATED (0x1 << 0)
> -#define DISASSEMBLY_RAW_INSN	(0x1 << 1)
> -#define DISASSEMBLY_OMIT_FNAME	(0x1 << 2)
> -#define DISASSEMBLY_FILENAME	(0x1 << 3)
> -#define DISASSEMBLY_OMIT_PC	(0x1 << 4)
> -#define DISASSEMBLY_SOURCE	(0x1 << 5)
> -#define DISASSEMBLY_SPECULATIVE	(0x1 << 6)
> +enum disassembly_flag {
> +  DISASSEMBLY_SOURCE_DEPRECATED = (1 << 0),
> +  DISASSEMBLY_RAW_INSN = (1 << 1),
> +  DISASSEMBLY_OMIT_FNAME = (1 << 2),
> +  DISASSEMBLY_FILENAME = (1 << 3),
> +  DISASSEMBLY_OMIT_PC = (1 << 4),
> +  DISASSEMBLY_SOURCE = (1 << 5),
> +  DISASSEMBLY_SPECULATIVE = (1 << 6),
> +};

Since we're touching this already, wouldn't it be slightly better to 
have all of the above in lowercase instead of uppercase? They look like 
#defined-ed constants otherwise.

Also, should we consider converting DISASSEMBLY_SOURCE_DEPRECATED or 
should we let it go? It was introduced in 2015.

Otherwise looks good to me.

Re: [PATCH] Introduce and use disassembly_flags

[Pedro Alves]

On 02/11/2017 03:12 AM, Simon Marchi wrote:
> +enum disassembly_flag {

Formatting, { on next line.

Thanks,
Pedro Alves

Re: [PATCH] Introduce and use disassembly_flags

[Simon Marchi]

On 2017-02-12 17:35, Yao Qi wrote:
> On Sat, Feb 11, 2017 at 3:12 AM, Simon Marchi <simon.marchi@polymtl.ca> 
> wrote:
>> @@ -1220,12 +1220,11 @@ disassemble_command (char *arg, int from_tty)
>>    CORE_ADDR low, high;
>>    const char *name;
>>    CORE_ADDR pc;
>> -  int flags;
>> +  disassembly_flags flags;
>>    const char *p;
>> 
>>    p = arg;
>>    name = NULL;
>> -  flags = 0;
>> 
> 
> We don't need to initialize 'flags' to zero?  The default construct of
> enum_flags
> doesn't nothing.

Hmm you're right.  Would it be a good idea to make the default 
constructor
initialize the value to 0?

Re: [PATCH] Introduce and use disassembly_flags

[Simon Marchi]

On 2017-02-13 15:02, Luis Machado wrote:
> On 02/10/2017 09:12 PM, Simon Marchi wrote:
>> diff --git a/gdb/disasm.h b/gdb/disasm.h
>> index 42c1f3af6d..bb7a7a0d5c 100644
>> --- a/gdb/disasm.h
>> +++ b/gdb/disasm.h
>> @@ -21,13 +21,17 @@
>> 
>>  #include "dis-asm.h"
>> 
>> -#define DISASSEMBLY_SOURCE_DEPRECATED (0x1 << 0)
>> -#define DISASSEMBLY_RAW_INSN	(0x1 << 1)
>> -#define DISASSEMBLY_OMIT_FNAME	(0x1 << 2)
>> -#define DISASSEMBLY_FILENAME	(0x1 << 3)
>> -#define DISASSEMBLY_OMIT_PC	(0x1 << 4)
>> -#define DISASSEMBLY_SOURCE	(0x1 << 5)
>> -#define DISASSEMBLY_SPECULATIVE	(0x1 << 6)
>> +enum disassembly_flag {
>> +  DISASSEMBLY_SOURCE_DEPRECATED = (1 << 0),
>> +  DISASSEMBLY_RAW_INSN = (1 << 1),
>> +  DISASSEMBLY_OMIT_FNAME = (1 << 2),
>> +  DISASSEMBLY_FILENAME = (1 << 3),
>> +  DISASSEMBLY_OMIT_PC = (1 << 4),
>> +  DISASSEMBLY_SOURCE = (1 << 5),
>> +  DISASSEMBLY_SPECULATIVE = (1 << 6),
>> +};
> 
> Since we're touching this already, wouldn't it be slightly better to
> have all of the above in lowercase instead of uppercase? They look
> like #defined-ed constants otherwise.

I didn't check all of them, but it looks like other enums used for flags 
are upper case.

> Also, should we consider converting DISASSEMBLY_SOURCE_DEPRECATED or
> should we let it go? It was introduced in 2015.

What do you mean by "converting"?

> Otherwise looks good to me.

Thanks,

Simon

Re: [PATCH] Introduce and use disassembly_flags

[Simon Marchi]

On 2017-02-15 06:15, Pedro Alves wrote:
> On 02/11/2017 03:12 AM, Simon Marchi wrote:
>> +enum disassembly_flag {
> 
> Formatting, { on next line.

Done, thanks.

Re: [PATCH] Introduce and use disassembly_flags

[Yao Qi]

On Sat, Feb 18, 2017 at 7:20 PM, Simon Marchi <simon.marchi@polymtl.ca> wrote:
>
> Hmm you're right.  Would it be a good idea to make the default constructor
> initialize the value to 0?

Yes, I think so.

-- 
Yao (齐尧)

Re: [PATCH] Introduce and use disassembly_flags

[Luis Machado]

On 02/18/2017 01:23 PM, Simon Marchi wrote:
> On 2017-02-13 15:02, Luis Machado wrote:
>> Also, should we consider converting DISASSEMBLY_SOURCE_DEPRECATED or
>> should we let it go? It was introduced in 2015.
>
> What do you mean by "converting"?

Since it is deprecated, do we need to convert/carry it over to the enum 
definitions? If not, it could be dropped.

I haven't looked much into this, so i'm not sure if we're still 
considering it acceptable to use this deprecated flag.

Re: [PATCH] Introduce and use disassembly_flags

[Simon Marchi]

On 2017-02-20 15:22, Luis Machado wrote:
> On 02/18/2017 01:23 PM, Simon Marchi wrote:
>> On 2017-02-13 15:02, Luis Machado wrote:
>>> Also, should we consider converting DISASSEMBLY_SOURCE_DEPRECATED or
>>> should we let it go? It was introduced in 2015.
>> 
>> What do you mean by "converting"?
> 
> Since it is deprecated, do we need to convert/carry it over to the
> enum definitions? If not, it could be dropped.
> 
> I haven't looked much into this, so i'm not sure if we're still
> considering it acceptable to use this deprecated flag.

This flag corresponds to the /m modifier of the disas command, which 
itself has been deprecated in 7.11.  As long as we don't remove the 
modifier, we can't remove the flag.

Simon

Re: [PATCH] Introduce and use disassembly_flags

[Luis Machado]

On 02/20/2017 02:37 PM, Simon Marchi wrote:
> On 2017-02-20 15:22, Luis Machado wrote:
>> On 02/18/2017 01:23 PM, Simon Marchi wrote:
>>> On 2017-02-13 15:02, Luis Machado wrote:
>>>> Also, should we consider converting DISASSEMBLY_SOURCE_DEPRECATED or
>>>> should we let it go? It was introduced in 2015.
>>>
>>> What do you mean by "converting"?
>>
>> Since it is deprecated, do we need to convert/carry it over to the
>> enum definitions? If not, it could be dropped.
>>
>> I haven't looked much into this, so i'm not sure if we're still
>> considering it acceptable to use this deprecated flag.
>
> This flag corresponds to the /m modifier of the disas command, which
> itself has been deprecated in 7.11.  As long as we don't remove the
> modifier, we can't remove the flag.

Fair enough. Thanks for the info.