[PATCH 7/7] Factor out memberptr printing code from c_val_print

[PATCH 7/7] Factor out memberptr printing code from c_val_print

[Simon Marchi]

gdb/ChangeLog:

	* c-valprint.c (c_val_print): Factor out memberptr printing code
	from c_val_print to ...
	(c_val_print_memberptr): ... this new function.
---
 gdb/c-valprint.c | 29 +++++++++++++++++++++++------
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/gdb/c-valprint.c b/gdb/c-valprint.c
index 0afd292..1cfe9d7 100644
--- a/gdb/c-valprint.c
+++ b/gdb/c-valprint.c
@@ -455,6 +455,26 @@ c_val_print_int (struct type *type, struct type *unresolved_type,
     }
 }
 
+/* c_val_print helper for TYPE_CODE_MEMBERPTR.  */
+
+static void
+c_val_print_memberptr (struct type *type, const gdb_byte *valaddr,
+		       int embedded_offset, CORE_ADDR address,
+		       struct ui_file *stream, int recurse,
+		       const struct value *original_value,
+		       const struct value_print_options *options)
+{
+  if (!options->format)
+    {
+      cp_print_class_member (valaddr + embedded_offset, type, stream, "&");
+    }
+  else
+    {
+      generic_val_print (type, valaddr, embedded_offset, address, stream,
+			 recurse, original_value, options, &c_decorations);
+    }
+}
+
 /* See val_print for a description of the various parameters of this
    function; they are identical.  */
 
@@ -500,12 +520,9 @@ c_val_print (struct type *type, const gdb_byte *valaddr,
       break;
 
     case TYPE_CODE_MEMBERPTR:
-      if (!options->format)
-	{
-	  cp_print_class_member (valaddr + embedded_offset, type, stream, "&");
-	  break;
-	}
-      /* FALLTHROUGH */
+      c_val_print_memberptr (type, valaddr, embedded_offset, address, stream,
+			     recurse, original_value, options);
+      break;
 
     case TYPE_CODE_REF:
     case TYPE_CODE_ENUM:
-- 
2.1.4

[PATCH 3/7] Factor out array printing code from c_val_print

[Simon Marchi]

gdb/ChangeLog:

	* c-valprint.c (c_valprint): Factor our array printing code from
	c_val_print to ...
	(c_val_print_array): ... this new function.
---
 gdb/c-valprint.c | 204 ++++++++++++++++++++++++++++++-------------------------
 1 file changed, 110 insertions(+), 94 deletions(-)

diff --git a/gdb/c-valprint.c b/gdb/c-valprint.c
index 41950ee..0b95a7f 100644
--- a/gdb/c-valprint.c
+++ b/gdb/c-valprint.c
@@ -225,120 +225,136 @@ print_unpacked_pointer (struct type *type, struct type *elttype,
     }
 }
 
-/* See val_print for a description of the various parameters of this
-   function; they are identical.  */
+/* c_val_print helper for TYPE_CODE_ARRAY.  */
 
-void
-c_val_print (struct type *type, const gdb_byte *valaddr,
-	     int embedded_offset, CORE_ADDR address,
-	     struct ui_file *stream, int recurse,
-	     const struct value *original_value,
-	     const struct value_print_options *options)
+static void
+c_val_print_array (struct type *type, const gdb_byte *valaddr,
+		   int embedded_offset, CORE_ADDR address,
+		   struct ui_file *stream, int recurse,
+		   const struct value *original_value,
+		   const struct value_print_options *options)
 {
-  struct gdbarch *gdbarch = get_type_arch (type);
-  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
-  unsigned len;
-  struct type *elttype, *unresolved_elttype;
-  struct type *unresolved_type = type;
-  unsigned eltlen;
-  CORE_ADDR addr;
+  struct type *unresolved_elttype = TYPE_TARGET_TYPE (type);
+  struct type *elttype = check_typedef (unresolved_elttype);
 
-  CHECK_TYPEDEF (type);
-  switch (TYPE_CODE (type))
+  if (TYPE_LENGTH (type) > 0 && TYPE_LENGTH (unresolved_elttype) > 0)
     {
-    unsigned int i = 0;	/* Number of characters printed.  */
-    case TYPE_CODE_ARRAY:
-      unresolved_elttype = TYPE_TARGET_TYPE (type);
-      elttype = check_typedef (unresolved_elttype);
-      if (TYPE_LENGTH (type) > 0 && TYPE_LENGTH (unresolved_elttype) > 0)
+      LONGEST low_bound, high_bound;
+      int eltlen, len;
+      struct gdbarch *gdbarch = get_type_arch (type);
+      enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+      unsigned int i = 0;	/* Number of characters printed.  */
+
+      if (!get_array_bounds (type, &low_bound, &high_bound))
+	error (_("Could not determine the array high bound"));
+
+      eltlen = TYPE_LENGTH (elttype);
+      len = high_bound - low_bound + 1;
+      if (options->prettyformat_arrays)
 	{
-          LONGEST low_bound, high_bound;
-
-          if (!get_array_bounds (type, &low_bound, &high_bound))
-            error (_("Could not determine the array high bound"));
+	  print_spaces_filtered (2 + 2 * recurse, stream);
+	}
 
-	  eltlen = TYPE_LENGTH (elttype);
-	  len = high_bound - low_bound + 1;
-	  if (options->prettyformat_arrays)
-	    {
-	      print_spaces_filtered (2 + 2 * recurse, stream);
-	    }
+      /* Print arrays of textual chars with a string syntax, as
+	 long as the entire array is valid.  */
+      if (c_textual_element_type (unresolved_elttype,
+				  options->format)
+	  && value_bytes_available (original_value, embedded_offset,
+				    TYPE_LENGTH (type))
+	  && !value_bits_any_optimized_out (original_value,
+					    TARGET_CHAR_BIT * embedded_offset,
+					    TARGET_CHAR_BIT * TYPE_LENGTH (type)))
+	{
+	  int force_ellipses = 0;
 
-	  /* Print arrays of textual chars with a string syntax, as
-	     long as the entire array is valid.  */
-          if (c_textual_element_type (unresolved_elttype,
-				      options->format)
-	      && value_bytes_available (original_value, embedded_offset,
-					TYPE_LENGTH (type))
-	      && !value_bits_any_optimized_out (original_value,
-						TARGET_CHAR_BIT * embedded_offset,
-						TARGET_CHAR_BIT * TYPE_LENGTH (type)))
+	  /* If requested, look for the first null char and only
+	     print elements up to it.  */
+	  if (options->stop_print_at_null)
 	    {
-	      int force_ellipses = 0;
-
-	      /* If requested, look for the first null char and only
-	         print elements up to it.  */
-	      if (options->stop_print_at_null)
+	      unsigned int temp_len;
+
+	      for (temp_len = 0;
+		   (temp_len < len
+		    && temp_len < options->print_max
+		    && extract_unsigned_integer (valaddr + embedded_offset
+						 + temp_len * eltlen,
+						 eltlen, byte_order) != 0);
+		   ++temp_len)
+		;
+
+	      /* Force LA_PRINT_STRING to print ellipses if
+		 we've printed the maximum characters and
+		 the next character is not \000.  */
+	      if (temp_len == options->print_max && temp_len < len)
 		{
-		  unsigned int temp_len;
-
-		  for (temp_len = 0;
-		       (temp_len < len
-			&& temp_len < options->print_max
-			&& extract_unsigned_integer (valaddr + embedded_offset
-						     + temp_len * eltlen,
-						     eltlen, byte_order) != 0);
-		       ++temp_len)
-		    ;
-
-		  /* Force LA_PRINT_STRING to print ellipses if
-		     we've printed the maximum characters and
-		     the next character is not \000.  */
-		  if (temp_len == options->print_max && temp_len < len)
-		    {
-		      ULONGEST val
-			= extract_unsigned_integer (valaddr + embedded_offset
-						    + temp_len * eltlen,
-						    eltlen, byte_order);
-		      if (val != 0)
-			force_ellipses = 1;
-		    }
-
-		  len = temp_len;
+		  ULONGEST val
+		    = extract_unsigned_integer (valaddr + embedded_offset
+						+ temp_len * eltlen,
+						eltlen, byte_order);
+		  if (val != 0)
+		    force_ellipses = 1;
 		}
 
-	      LA_PRINT_STRING (stream, unresolved_elttype,
-			       valaddr + embedded_offset, len,
-			       NULL, force_ellipses, options);
-	      i = len;
+	      len = temp_len;
+	    }
+
+	  LA_PRINT_STRING (stream, unresolved_elttype,
+			   valaddr + embedded_offset, len,
+			   NULL, force_ellipses, options);
+	  i = len;
+	}
+      else
+	{
+	  fprintf_filtered (stream, "{");
+	  /* If this is a virtual function table, print the 0th
+	     entry specially, and the rest of the members
+	     normally.  */
+	  if (cp_is_vtbl_ptr_type (elttype))
+	    {
+	      i = 1;
+	      fprintf_filtered (stream, _("%d vtable entries"),
+				len - 1);
 	    }
 	  else
 	    {
-	      fprintf_filtered (stream, "{");
-	      /* If this is a virtual function table, print the 0th
-	         entry specially, and the rest of the members
-	         normally.  */
-	      if (cp_is_vtbl_ptr_type (elttype))
-		{
-		  i = 1;
-		  fprintf_filtered (stream, _("%d vtable entries"),
-				    len - 1);
-		}
-	      else
-		{
-		  i = 0;
-		}
-	      val_print_array_elements (type, valaddr, embedded_offset,
-					address, stream,
-					recurse, original_value, options, i);
-	      fprintf_filtered (stream, "}");
+	      i = 0;
 	    }
-	  break;
+	  val_print_array_elements (type, valaddr, embedded_offset,
+				    address, stream,
+				    recurse, original_value, options, i);
+	  fprintf_filtered (stream, "}");
 	}
+    }
+  else
+    {
       /* Array of unspecified length: treat like pointer to first elt.  */
       print_unpacked_pointer (type, elttype, unresolved_elttype, valaddr,
 			      embedded_offset, address + embedded_offset,
 			      stream, recurse, options);
+    }
+}
+
+/* See val_print for a description of the various parameters of this
+   function; they are identical.  */
+
+void
+c_val_print (struct type *type, const gdb_byte *valaddr,
+	     int embedded_offset, CORE_ADDR address,
+	     struct ui_file *stream, int recurse,
+	     const struct value *original_value,
+	     const struct value_print_options *options)
+{
+  struct gdbarch *gdbarch = get_type_arch (type);
+  struct type *elttype, *unresolved_elttype;
+  struct type *unresolved_type = type;
+  CORE_ADDR addr;
+
+  CHECK_TYPEDEF (type);
+  switch (TYPE_CODE (type))
+    {
+    case TYPE_CODE_ARRAY:
+      c_val_print_array (type, valaddr, embedded_offset, address, stream,
+			 recurse, original_value, options);
       break;
 
     case TYPE_CODE_METHODPTR:
-- 
2.1.4

[PATCH 0/7] Split c_val_print

[Simon Marchi]

I think that c_val_print deserves to be split up in smaller, more
manageable chunks.  This series does that, by simply factoring out each
case of the big switch.  In some cases, a bit of modifications were
necessary where fallthrough between cases or goto were used, but
otherwise the code stays the same.

Simon Marchi (7):
  Remove unneeded variable assignment
  Factor out print_unpacked_pointer from c_val_print
  Factor out array printing code from c_val_print
  Factor out pointer printing code from c_val_print
  Factor out struct and union printing code from c_val_print
  Factor out int printing code from c_val_print
  Factor out memberptr printing code from c_val_print

 gdb/c-valprint.c | 612 ++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 352 insertions(+), 260 deletions(-)

-- 
2.1.4

[PATCH 6/7] Factor out int printing code from c_val_print

[Simon Marchi]

gdb/ChangeLog:

	* c-valprint.c (c_val_print): Factor out int printing code to ...
	(c_val_print_int): ... this new function.
---
 gdb/c-valprint.c | 60 +++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 36 insertions(+), 24 deletions(-)

diff --git a/gdb/c-valprint.c b/gdb/c-valprint.c
index d950de3..0afd292 100644
--- a/gdb/c-valprint.c
+++ b/gdb/c-valprint.c
@@ -421,6 +421,40 @@ c_val_print_union (struct type *type, const gdb_byte *valaddr,
     }
 }
 
+/* c_val_print helper for TYPE_CODE_INT.  */
+
+static void
+c_val_print_int (struct type *type, struct type *unresolved_type,
+		 const gdb_byte *valaddr, int embedded_offset,
+		 struct ui_file *stream, const struct value *original_value,
+		 const struct value_print_options *options)
+{
+  if (options->format || options->output_format)
+    {
+      struct value_print_options opts = *options;
+
+      opts.format = (options->format ? options->format
+		     : options->output_format);
+      val_print_scalar_formatted (type, valaddr, embedded_offset,
+				  original_value, &opts, 0, stream);
+    }
+  else
+    {
+      val_print_type_code_int (type, valaddr + embedded_offset,
+			       stream);
+      /* C and C++ has no single byte int type, char is used
+	 instead.  Since we don't know whether the value is really
+	 intended to be used as an integer or a character, print
+	 the character equivalent as well.  */
+      if (c_textual_element_type (unresolved_type, options->format))
+	{
+	  fputs_filtered (" ", stream);
+	  LA_PRINT_CHAR (unpack_long (type, valaddr + embedded_offset),
+			 unresolved_type, stream);
+	}
+    }
+}
+
 /* See val_print for a description of the various parameters of this
    function; they are identical.  */
 
@@ -461,30 +495,8 @@ c_val_print (struct type *type, const gdb_byte *valaddr,
       break;
 
     case TYPE_CODE_INT:
-      if (options->format || options->output_format)
-	{
-	  struct value_print_options opts = *options;
-
-	  opts.format = (options->format ? options->format
-			 : options->output_format);
-	  val_print_scalar_formatted (type, valaddr, embedded_offset,
-				      original_value, &opts, 0, stream);
-	}
-      else
-	{
-	  val_print_type_code_int (type, valaddr + embedded_offset,
-				   stream);
-	  /* C and C++ has no single byte int type, char is used
-	     instead.  Since we don't know whether the value is really
-	     intended to be used as an integer or a character, print
-	     the character equivalent as well.  */
-	  if (c_textual_element_type (unresolved_type, options->format))
-	    {
-	      fputs_filtered (" ", stream);
-	      LA_PRINT_CHAR (unpack_long (type, valaddr + embedded_offset),
-			     unresolved_type, stream);
-	    }
-	}
+      c_val_print_int (type, unresolved_type, valaddr, embedded_offset, stream,
+		       original_value, options);
       break;
 
     case TYPE_CODE_MEMBERPTR:
-- 
2.1.4

[PATCH 2/7] Factor out print_unpacked_pointer from c_val_print

[Simon Marchi]

Turn this code into a function, instead of a goto.

gdb/ChangeLog:

	* c-valprint.c (c_val_print): Factor out pointer printing code
	to ...
	(print_unpacked_pointer): ... this new function.
---
 gdb/c-valprint.c | 205 ++++++++++++++++++++++++++++---------------------------
 1 file changed, 106 insertions(+), 99 deletions(-)

diff --git a/gdb/c-valprint.c b/gdb/c-valprint.c
index e8a2fc7..41950ee 100644
--- a/gdb/c-valprint.c
+++ b/gdb/c-valprint.c
@@ -127,6 +127,104 @@ static const struct generic_val_print_decorations c_decorations =
   "void"
 };
 
+/* Print a pointer based on the type of its target.
+ *
+ * Arguments to this functions are roughly the same as those in c_val_print.
+ * A difference is that ADDRESS is the address to print, with embedded_offset
+ * already added.  UNRESOLVED_ELTTYPE and ELTTYPE represent the pointed type,
+ * respectively before and after check_typedef.  */
+
+static void
+print_unpacked_pointer (struct type *type, struct type *elttype,
+			struct type *unresolved_elttype,
+			const gdb_byte *valaddr, int embedded_offset,
+			CORE_ADDR address, struct ui_file *stream, int recurse,
+			const struct value_print_options *options)
+{
+  int want_space = 0;
+  struct gdbarch *gdbarch = get_type_arch (type);
+
+  if (TYPE_CODE (elttype) == TYPE_CODE_FUNC)
+    {
+      /* Try to print what function it points to.  */
+      print_function_pointer_address (options, gdbarch, address, stream);
+      return;
+    }
+
+  if (options->symbol_print)
+    want_space = print_address_demangle (options, gdbarch, address, stream,
+					 demangle);
+  else if (options->addressprint)
+    {
+      fputs_filtered (paddress (gdbarch, address), stream);
+      want_space = 1;
+    }
+
+  /* For a pointer to a textual type, also print the string
+     pointed to, unless pointer is null.  */
+
+  if (c_textual_element_type (unresolved_elttype, options->format)
+      && address != 0)
+    {
+      if (want_space)
+	fputs_filtered (" ", stream);
+      val_print_string (unresolved_elttype, NULL, address, -1, stream, options);
+    }
+  else if (cp_is_vtbl_member (type))
+    {
+      /* Print vtbl's nicely.  */
+      CORE_ADDR vt_address = unpack_pointer (type, valaddr + embedded_offset);
+      struct bound_minimal_symbol msymbol =
+	lookup_minimal_symbol_by_pc (vt_address);
+
+      /* If 'symbol_print' is set, we did the work above.  */
+      if (!options->symbol_print
+	  && (msymbol.minsym != NULL)
+	  && (vt_address == BMSYMBOL_VALUE_ADDRESS (msymbol)))
+	{
+	  if (want_space)
+	    fputs_filtered (" ", stream);
+	  fputs_filtered (" <", stream);
+	  fputs_filtered (MSYMBOL_PRINT_NAME (msymbol.minsym), stream);
+	  fputs_filtered (">", stream);
+	  want_space = 1;
+	}
+
+      if (vt_address && options->vtblprint)
+	{
+	  struct value *vt_val;
+	  struct symbol *wsym = (struct symbol *) NULL;
+	  struct type *wtype;
+	  struct block *block = (struct block *) NULL;
+	  struct field_of_this_result is_this_fld;
+
+	  if (want_space)
+	    fputs_filtered (" ", stream);
+
+	  if (msymbol.minsym != NULL)
+	    wsym = lookup_symbol (MSYMBOL_LINKAGE_NAME(msymbol.minsym), block,
+				  VAR_DOMAIN, &is_this_fld);
+
+	  if (wsym)
+	    {
+	      wtype = SYMBOL_TYPE (wsym);
+	    }
+	  else
+	    {
+	      wtype = unresolved_elttype;
+	    }
+	  vt_val = value_at (wtype, vt_address);
+	  common_val_print (vt_val, stream, recurse + 1, options,
+			    current_language);
+	  if (options->prettyformat)
+	    {
+	      fprintf_filtered (stream, "\n");
+	      print_spaces_filtered (2 + 2 * recurse, stream);
+	    }
+	}
+    }
+}
+
 /* See val_print for a description of the various parameters of this
    function; they are identical.  */
 
@@ -237,10 +335,11 @@ c_val_print (struct type *type, const gdb_byte *valaddr,
 	    }
 	  break;
 	}
-      /* Array of unspecified length: treat like pointer to first
-	 elt.  */
-      addr = address + embedded_offset;
-      goto print_unpacked_pointer;
+      /* Array of unspecified length: treat like pointer to first elt.  */
+      print_unpacked_pointer (type, elttype, unresolved_elttype, valaddr,
+			      embedded_offset, address + embedded_offset,
+			      stream, recurse, options);
+      break;
 
     case TYPE_CODE_METHODPTR:
       cplus_print_method_ptr (valaddr + embedded_offset, type, stream);
@@ -267,101 +366,9 @@ c_val_print (struct type *type, const gdb_byte *valaddr,
 	}
       unresolved_elttype = TYPE_TARGET_TYPE (type);
       elttype = check_typedef (unresolved_elttype);
-	{
-	  int want_space;
-
-	  addr = unpack_pointer (type, valaddr + embedded_offset);
-	print_unpacked_pointer:
-
-	  want_space = 0;
-
-	  if (TYPE_CODE (elttype) == TYPE_CODE_FUNC)
-	    {
-	      /* Try to print what function it points to.  */
-	      print_function_pointer_address (options, gdbarch, addr, stream);
-	      return;
-	    }
-
-	  if (options->symbol_print)
-	    want_space = print_address_demangle (options, gdbarch, addr,
-						 stream, demangle);
-	  else if (options->addressprint)
-	    {
-	      fputs_filtered (paddress (gdbarch, addr), stream);
-	      want_space = 1;
-	    }
-
-	  /* For a pointer to a textual type, also print the string
-	     pointed to, unless pointer is null.  */
-
-	  if (c_textual_element_type (unresolved_elttype,
-				      options->format)
-	      && addr != 0)
-	    {
-	      if (want_space)
-		fputs_filtered (" ", stream);
-	      val_print_string (unresolved_elttype, NULL,
-				addr, -1,
-				stream, options);
-	    }
-	  else if (cp_is_vtbl_member (type))
-	    {
-	      /* Print vtbl's nicely.  */
-	      CORE_ADDR vt_address = unpack_pointer (type,
-						     valaddr
-						     + embedded_offset);
-	      struct bound_minimal_symbol msymbol =
-		lookup_minimal_symbol_by_pc (vt_address);
-
-	      /* If 'symbol_print' is set, we did the work above.  */
-	      if (!options->symbol_print
-		  && (msymbol.minsym != NULL)
-		  && (vt_address == BMSYMBOL_VALUE_ADDRESS (msymbol)))
-		{
-		  if (want_space)
-		    fputs_filtered (" ", stream);
-		  fputs_filtered (" <", stream);
-		  fputs_filtered (MSYMBOL_PRINT_NAME (msymbol.minsym), stream);
-		  fputs_filtered (">", stream);
-		  want_space = 1;
-		}
-
-	      if (vt_address && options->vtblprint)
-		{
-		  struct value *vt_val;
-		  struct symbol *wsym = (struct symbol *) NULL;
-		  struct type *wtype;
-		  struct block *block = (struct block *) NULL;
-		  struct field_of_this_result is_this_fld;
-
-		  if (want_space)
-		    fputs_filtered (" ", stream);
-
-		  if (msymbol.minsym != NULL)
-		    wsym = lookup_symbol (MSYMBOL_LINKAGE_NAME (msymbol.minsym),
-					  block, VAR_DOMAIN,
-					  &is_this_fld);
-
-		  if (wsym)
-		    {
-		      wtype = SYMBOL_TYPE (wsym);
-		    }
-		  else
-		    {
-		      wtype = unresolved_elttype;
-		    }
-		  vt_val = value_at (wtype, vt_address);
-		  common_val_print (vt_val, stream, recurse + 1,
-				    options, current_language);
-		  if (options->prettyformat)
-		    {
-		      fprintf_filtered (stream, "\n");
-		      print_spaces_filtered (2 + 2 * recurse, stream);
-		    }
-		}
-	    }
-	  return;
-	}
+      addr = unpack_pointer (type, valaddr + embedded_offset);
+      print_unpacked_pointer (type, elttype, unresolved_elttype, valaddr,
+			      embedded_offset, addr, stream, recurse, options);
       break;
 
     case TYPE_CODE_UNION:
-- 
2.1.4

[PATCH 4/7] Factor out pointer printing code from c_val_print

[Simon Marchi]

gdb/ChangeLog:

	* c-valprint.c (c_val_print): Factor out pointer printing code
	to ...
	(c_val_print_ptr): ... this new function.
---
 gdb/c-valprint.c | 62 +++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 37 insertions(+), 25 deletions(-)

diff --git a/gdb/c-valprint.c b/gdb/c-valprint.c
index 0b95a7f..9f68815 100644
--- a/gdb/c-valprint.c
+++ b/gdb/c-valprint.c
@@ -334,6 +334,41 @@ c_val_print_array (struct type *type, const gdb_byte *valaddr,
     }
 }
 
+/* c_val_print helper for TYPE_CODE_PTR.  */
+
+static void
+c_val_print_ptr (struct type *type, const gdb_byte *valaddr,
+		 int embedded_offset, struct ui_file *stream, int recurse,
+		 const struct value *original_value,
+		 const struct value_print_options *options)
+{
+  if (options->format && options->format != 's')
+    {
+      val_print_scalar_formatted (type, valaddr, embedded_offset,
+				  original_value, options, 0, stream);
+    }
+  else if (options->vtblprint && cp_is_vtbl_ptr_type (type))
+    {
+      /* Print the unmangled name if desired.  */
+      /* Print vtable entry - we only get here if we ARE using
+	 -fvtable_thunks.  (Otherwise, look under
+	 TYPE_CODE_STRUCT.)  */
+      CORE_ADDR addr
+	= extract_typed_address (valaddr + embedded_offset, type);
+      struct gdbarch *gdbarch = get_type_arch (type);
+
+      print_function_pointer_address (options, gdbarch, addr, stream);
+    }
+  else
+    {
+      struct type *unresolved_elttype = TYPE_TARGET_TYPE (type);
+      struct type *elttype = check_typedef (unresolved_elttype);
+      CORE_ADDR addr = unpack_pointer (type, valaddr + embedded_offset);
+      print_unpacked_pointer (type, elttype, unresolved_elttype, valaddr,
+			      embedded_offset, addr, stream, recurse, options);
+    }
+}
+
 /* See val_print for a description of the various parameters of this
    function; they are identical.  */
 
@@ -345,9 +380,7 @@ c_val_print (struct type *type, const gdb_byte *valaddr,
 	     const struct value_print_options *options)
 {
   struct gdbarch *gdbarch = get_type_arch (type);
-  struct type *elttype, *unresolved_elttype;
   struct type *unresolved_type = type;
-  CORE_ADDR addr;
 
   CHECK_TYPEDEF (type);
   switch (TYPE_CODE (type))
@@ -362,29 +395,8 @@ c_val_print (struct type *type, const gdb_byte *valaddr,
       break;
 
     case TYPE_CODE_PTR:
-      if (options->format && options->format != 's')
-	{
-	  val_print_scalar_formatted (type, valaddr, embedded_offset,
-				      original_value, options, 0, stream);
-	  break;
-	}
-      if (options->vtblprint && cp_is_vtbl_ptr_type (type))
-	{
-	  /* Print the unmangled name if desired.  */
-	  /* Print vtable entry - we only get here if we ARE using
-	     -fvtable_thunks.  (Otherwise, look under
-	     TYPE_CODE_STRUCT.)  */
-	  CORE_ADDR addr
-	    = extract_typed_address (valaddr + embedded_offset, type);
-
-	  print_function_pointer_address (options, gdbarch, addr, stream);
-	  break;
-	}
-      unresolved_elttype = TYPE_TARGET_TYPE (type);
-      elttype = check_typedef (unresolved_elttype);
-      addr = unpack_pointer (type, valaddr + embedded_offset);
-      print_unpacked_pointer (type, elttype, unresolved_elttype, valaddr,
-			      embedded_offset, addr, stream, recurse, options);
+      c_val_print_ptr (type, valaddr, embedded_offset, stream, recurse,
+		       original_value, options);
       break;
 
     case TYPE_CODE_UNION:
-- 
2.1.4

[PATCH 5/7] Factor out struct and union printing code from c_val_print

[Simon Marchi]

gdb/ChangeLog:

	* c-valprint.c (c_val_print): Factor out struct and union
	printing code to ...
	(c_val_print_struct): ... this new function ...
	(c_val_print_union): ... and this new function.
---
 gdb/c-valprint.c | 88 +++++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 58 insertions(+), 30 deletions(-)

diff --git a/gdb/c-valprint.c b/gdb/c-valprint.c
index 9f68815..d950de3 100644
--- a/gdb/c-valprint.c
+++ b/gdb/c-valprint.c
@@ -369,6 +369,58 @@ c_val_print_ptr (struct type *type, const gdb_byte *valaddr,
     }
 }
 
+/* c_val_print helper for TYPE_CODE_STRUCT.  */
+
+static void
+c_val_print_struct (struct type *type, const gdb_byte *valaddr,
+		    int embedded_offset, CORE_ADDR address,
+		    struct ui_file *stream, int recurse,
+		    const struct value *original_value,
+		    const struct value_print_options *options)
+{
+  if (options->vtblprint && cp_is_vtbl_ptr_type (type))
+    {
+      /* Print the unmangled name if desired.  */
+      /* Print vtable entry - we only get here if NOT using
+	 -fvtable_thunks.  (Otherwise, look under
+	 TYPE_CODE_PTR.)  */
+      struct gdbarch *gdbarch = get_type_arch (type);
+      int offset = (embedded_offset
+		    + TYPE_FIELD_BITPOS (type,
+					 VTBL_FNADDR_OFFSET) / 8);
+      struct type *field_type = TYPE_FIELD_TYPE (type, VTBL_FNADDR_OFFSET);
+      CORE_ADDR addr = extract_typed_address (valaddr + offset, field_type);
+
+      print_function_pointer_address (options, gdbarch, addr, stream);
+    }
+  else
+    cp_print_value_fields_rtti (type, valaddr,
+				embedded_offset, address,
+				stream, recurse,
+				original_value, options,
+				NULL, 0);
+}
+
+/* c_val_print helper for TYPE_CODE_UNION.  */
+
+static void
+c_val_print_union (struct type *type, const gdb_byte *valaddr,
+		   int embedded_offset, CORE_ADDR address,
+		   struct ui_file *stream, int recurse,
+		   const struct value *original_value,
+		   const struct value_print_options *options)
+{
+  if (recurse && !options->unionprint)
+    {
+      fprintf_filtered (stream, "{...}");
+     }
+  else
+    {
+      c_val_print_struct (type, valaddr, embedded_offset, address, stream,
+			  recurse, original_value, options);
+    }
+}
+
 /* See val_print for a description of the various parameters of this
    function; they are identical.  */
 
@@ -379,7 +431,6 @@ c_val_print (struct type *type, const gdb_byte *valaddr,
 	     const struct value *original_value,
 	     const struct value_print_options *options)
 {
-  struct gdbarch *gdbarch = get_type_arch (type);
   struct type *unresolved_type = type;
 
   CHECK_TYPEDEF (type);
@@ -400,36 +451,13 @@ c_val_print (struct type *type, const gdb_byte *valaddr,
       break;
 
     case TYPE_CODE_UNION:
-      if (recurse && !options->unionprint)
-	{
-	  fprintf_filtered (stream, "{...}");
-	  break;
-	}
-      /* Fall through.  */
+      c_val_print_union (type, valaddr, embedded_offset, address, stream,
+			 recurse, original_value, options);
+      break;
+
     case TYPE_CODE_STRUCT:
-      /*FIXME: Abstract this away.  */
-      if (options->vtblprint && cp_is_vtbl_ptr_type (type))
-	{
-	  /* Print the unmangled name if desired.  */
-	  /* Print vtable entry - we only get here if NOT using
-	     -fvtable_thunks.  (Otherwise, look under
-	     TYPE_CODE_PTR.)  */
-	  int offset = (embedded_offset
-			+ TYPE_FIELD_BITPOS (type,
-					     VTBL_FNADDR_OFFSET) / 8);
-	  struct type *field_type = TYPE_FIELD_TYPE (type,
-						     VTBL_FNADDR_OFFSET);
-	  CORE_ADDR addr
-	    = extract_typed_address (valaddr + offset, field_type);
-
-	  print_function_pointer_address (options, gdbarch, addr, stream);
-	}
-      else
-	cp_print_value_fields_rtti (type, valaddr,
-				    embedded_offset, address,
-				    stream, recurse,
-				    original_value, options,
-				    NULL, 0);
+      c_val_print_struct (type, valaddr, embedded_offset, address, stream,
+			  recurse, original_value, options);
       break;
 
     case TYPE_CODE_INT:
-- 
2.1.4

[PATCH 1/7] Remove unneeded variable assignment

[Simon Marchi]

The assignment to i in the TYPE_CODE_PTR section is not useful.
Removing it allows to move i in a narrower scope, which will help
things somewhere in the next patches.

gdb/ChangeLog:

	* c-valprint.c (c_val_print): Remove an assignment to i and move
	its declaration.
---
 gdb/c-valprint.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/gdb/c-valprint.c b/gdb/c-valprint.c
index 8d8b744..e8a2fc7 100644
--- a/gdb/c-valprint.c
+++ b/gdb/c-valprint.c
@@ -139,7 +139,6 @@ c_val_print (struct type *type, const gdb_byte *valaddr,
 {
   struct gdbarch *gdbarch = get_type_arch (type);
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
-  unsigned int i = 0;	/* Number of characters printed.  */
   unsigned len;
   struct type *elttype, *unresolved_elttype;
   struct type *unresolved_type = type;
@@ -149,6 +148,7 @@ c_val_print (struct type *type, const gdb_byte *valaddr,
   CHECK_TYPEDEF (type);
   switch (TYPE_CODE (type))
     {
+    unsigned int i = 0;	/* Number of characters printed.  */
     case TYPE_CODE_ARRAY:
       unresolved_elttype = TYPE_TARGET_TYPE (type);
       elttype = check_typedef (unresolved_elttype);
@@ -300,9 +300,9 @@ c_val_print (struct type *type, const gdb_byte *valaddr,
 	    {
 	      if (want_space)
 		fputs_filtered (" ", stream);
-	      i = val_print_string (unresolved_elttype, NULL,
-				    addr, -1,
-				    stream, options);
+	      val_print_string (unresolved_elttype, NULL,
+				addr, -1,
+				stream, options);
 	    }
 	  else if (cp_is_vtbl_member (type))
 	    {
-- 
2.1.4

Re: [PATCH 0/7] Split c_val_print

[Pedro Alves]

On 07/08/2015 10:07 PM, Simon Marchi wrote:
> I think that c_val_print deserves to be split up in smaller, more
> manageable chunks.  This series does that, by simply factoring out each
> case of the big switch.  In some cases, a bit of modifications were
> necessary where fallthrough between cases or goto were used, but
> otherwise the code stays the same.

Nice!  A couple issues need to be fixed and this is good to go.

 - The intro comment to print_unpacked_pointer in patch 2
   uses the wrong format (leading * in each line?).

 - For convention, make sure there's a line break after
   each variable declaration.

Fix these and push.

Thanks,
Pedro Alves

Re: [PATCH 0/7] Split c_val_print

[Simon Marchi]

On 15-07-09 06:53 AM, Pedro Alves wrote:
> Nice!  A couple issues need to be fixed and this is good to go.
> 
>  - The intro comment to print_unpacked_pointer in patch 2
>    uses the wrong format (leading * in each line?).

Ah, that's an artifact from CDT :). Fixed.

>  - For convention, make sure there's a line break after
>    each variable declaration.

You mean, after each block of variable declarations, and not
after each single variable declaration I suppose?

I only found one instance, in c_val_print_ptr.

> Fix these and push.

Done, thanks.


> Thanks,
> Pedro Alves

Re: [PATCH 2/7] Factor out print_unpacked_pointer from c_val_print

[Simon Marchi]

On 15-07-08 05:07 PM, Simon Marchi wrote:
> Turn this code into a function, instead of a goto.
> 
> gdb/ChangeLog:
> 
> 	* c-valprint.c (c_val_print): Factor out pointer printing code
> 	to ...
> 	(print_unpacked_pointer): ... this new function.
> ---
>  gdb/c-valprint.c | 205 ++++++++++++++++++++++++++++---------------------------
>  1 file changed, 106 insertions(+), 99 deletions(-)
> 
> diff --git a/gdb/c-valprint.c b/gdb/c-valprint.c
> index e8a2fc7..41950ee 100644
> --- a/gdb/c-valprint.c
> +++ b/gdb/c-valprint.c
> @@ -127,6 +127,104 @@ static const struct generic_val_print_decorations c_decorations =
>    "void"
>  };
>  
> +/* Print a pointer based on the type of its target.
> + *
> + * Arguments to this functions are roughly the same as those in c_val_print.
> + * A difference is that ADDRESS is the address to print, with embedded_offset
> + * already added.  UNRESOLVED_ELTTYPE and ELTTYPE represent the pointed type,
> + * respectively before and after check_typedef.  */
> +
> +static void
> +print_unpacked_pointer (struct type *type, struct type *elttype,
> +			struct type *unresolved_elttype,
> +			const gdb_byte *valaddr, int embedded_offset,
> +			CORE_ADDR address, struct ui_file *stream, int recurse,
> +			const struct value_print_options *options)
> +{
> +  int want_space = 0;
> +  struct gdbarch *gdbarch = get_type_arch (type);
> +
> +  if (TYPE_CODE (elttype) == TYPE_CODE_FUNC)
> +    {
> +      /* Try to print what function it points to.  */
> +      print_function_pointer_address (options, gdbarch, address, stream);
> +      return;
> +    }
> +
> +  if (options->symbol_print)
> +    want_space = print_address_demangle (options, gdbarch, address, stream,
> +					 demangle);
> +  else if (options->addressprint)
> +    {
> +      fputs_filtered (paddress (gdbarch, address), stream);
> +      want_space = 1;
> +    }
> +
> +  /* For a pointer to a textual type, also print the string
> +     pointed to, unless pointer is null.  */
> +
> +  if (c_textual_element_type (unresolved_elttype, options->format)
> +      && address != 0)
> +    {
> +      if (want_space)
> +	fputs_filtered (" ", stream);
> +      val_print_string (unresolved_elttype, NULL, address, -1, stream, options);
> +    }
> +  else if (cp_is_vtbl_member (type))
> +    {
> +      /* Print vtbl's nicely.  */
> +      CORE_ADDR vt_address = unpack_pointer (type, valaddr + embedded_offset);
> +      struct bound_minimal_symbol msymbol =
> +	lookup_minimal_symbol_by_pc (vt_address);
> +
> +      /* If 'symbol_print' is set, we did the work above.  */
> +      if (!options->symbol_print
> +	  && (msymbol.minsym != NULL)
> +	  && (vt_address == BMSYMBOL_VALUE_ADDRESS (msymbol)))
> +	{
> +	  if (want_space)
> +	    fputs_filtered (" ", stream);
> +	  fputs_filtered (" <", stream);
> +	  fputs_filtered (MSYMBOL_PRINT_NAME (msymbol.minsym), stream);
> +	  fputs_filtered (">", stream);
> +	  want_space = 1;
> +	}
> +
> +      if (vt_address && options->vtblprint)
> +	{
> +	  struct value *vt_val;
> +	  struct symbol *wsym = (struct symbol *) NULL;
> +	  struct type *wtype;
> +	  struct block *block = (struct block *) NULL;
> +	  struct field_of_this_result is_this_fld;
> +
> +	  if (want_space)
> +	    fputs_filtered (" ", stream);
> +
> +	  if (msymbol.minsym != NULL)
> +	    wsym = lookup_symbol (MSYMBOL_LINKAGE_NAME(msymbol.minsym), block,
> +				  VAR_DOMAIN, &is_this_fld);
> +
> +	  if (wsym)
> +	    {
> +	      wtype = SYMBOL_TYPE (wsym);
> +	    }
> +	  else
> +	    {
> +	      wtype = unresolved_elttype;
> +	    }
> +	  vt_val = value_at (wtype, vt_address);
> +	  common_val_print (vt_val, stream, recurse + 1, options,
> +			    current_language);
> +	  if (options->prettyformat)
> +	    {
> +	      fprintf_filtered (stream, "\n");
> +	      print_spaces_filtered (2 + 2 * recurse, stream);
> +	    }
> +	}
> +    }
> +}
> +
>  /* See val_print for a description of the various parameters of this
>     function; they are identical.  */
>  
> @@ -237,10 +335,11 @@ c_val_print (struct type *type, const gdb_byte *valaddr,
>  	    }
>  	  break;
>  	}
> -      /* Array of unspecified length: treat like pointer to first
> -	 elt.  */
> -      addr = address + embedded_offset;
> -      goto print_unpacked_pointer;
> +      /* Array of unspecified length: treat like pointer to first elt.  */
> +      print_unpacked_pointer (type, elttype, unresolved_elttype, valaddr,
> +			      embedded_offset, address + embedded_offset,
> +			      stream, recurse, options);
> +      break;
>  
>      case TYPE_CODE_METHODPTR:
>        cplus_print_method_ptr (valaddr + embedded_offset, type, stream);
> @@ -267,101 +366,9 @@ c_val_print (struct type *type, const gdb_byte *valaddr,
>  	}
>        unresolved_elttype = TYPE_TARGET_TYPE (type);
>        elttype = check_typedef (unresolved_elttype);
> -	{
> -	  int want_space;
> -
> -	  addr = unpack_pointer (type, valaddr + embedded_offset);
> -	print_unpacked_pointer:
> -
> -	  want_space = 0;
> -
> -	  if (TYPE_CODE (elttype) == TYPE_CODE_FUNC)
> -	    {
> -	      /* Try to print what function it points to.  */
> -	      print_function_pointer_address (options, gdbarch, addr, stream);
> -	      return;
> -	    }
> -
> -	  if (options->symbol_print)
> -	    want_space = print_address_demangle (options, gdbarch, addr,
> -						 stream, demangle);
> -	  else if (options->addressprint)
> -	    {
> -	      fputs_filtered (paddress (gdbarch, addr), stream);
> -	      want_space = 1;
> -	    }
> -
> -	  /* For a pointer to a textual type, also print the string
> -	     pointed to, unless pointer is null.  */
> -
> -	  if (c_textual_element_type (unresolved_elttype,
> -				      options->format)
> -	      && addr != 0)
> -	    {
> -	      if (want_space)
> -		fputs_filtered (" ", stream);
> -	      val_print_string (unresolved_elttype, NULL,
> -				addr, -1,
> -				stream, options);
> -	    }
> -	  else if (cp_is_vtbl_member (type))
> -	    {
> -	      /* Print vtbl's nicely.  */
> -	      CORE_ADDR vt_address = unpack_pointer (type,
> -						     valaddr
> -						     + embedded_offset);
> -	      struct bound_minimal_symbol msymbol =
> -		lookup_minimal_symbol_by_pc (vt_address);
> -
> -	      /* If 'symbol_print' is set, we did the work above.  */
> -	      if (!options->symbol_print
> -		  && (msymbol.minsym != NULL)
> -		  && (vt_address == BMSYMBOL_VALUE_ADDRESS (msymbol)))
> -		{
> -		  if (want_space)
> -		    fputs_filtered (" ", stream);
> -		  fputs_filtered (" <", stream);
> -		  fputs_filtered (MSYMBOL_PRINT_NAME (msymbol.minsym), stream);
> -		  fputs_filtered (">", stream);
> -		  want_space = 1;
> -		}
> -
> -	      if (vt_address && options->vtblprint)
> -		{
> -		  struct value *vt_val;
> -		  struct symbol *wsym = (struct symbol *) NULL;
> -		  struct type *wtype;
> -		  struct block *block = (struct block *) NULL;
> -		  struct field_of_this_result is_this_fld;
> -
> -		  if (want_space)
> -		    fputs_filtered (" ", stream);
> -
> -		  if (msymbol.minsym != NULL)
> -		    wsym = lookup_symbol (MSYMBOL_LINKAGE_NAME (msymbol.minsym),
> -					  block, VAR_DOMAIN,
> -					  &is_this_fld);
> -
> -		  if (wsym)
> -		    {
> -		      wtype = SYMBOL_TYPE (wsym);
> -		    }
> -		  else
> -		    {
> -		      wtype = unresolved_elttype;
> -		    }
> -		  vt_val = value_at (wtype, vt_address);
> -		  common_val_print (vt_val, stream, recurse + 1,
> -				    options, current_language);
> -		  if (options->prettyformat)
> -		    {
> -		      fprintf_filtered (stream, "\n");
> -		      print_spaces_filtered (2 + 2 * recurse, stream);
> -		    }
> -		}
> -	    }
> -	  return;
> -	}
> +      addr = unpack_pointer (type, valaddr + embedded_offset);
> +      print_unpacked_pointer (type, elttype, unresolved_elttype, valaddr,
> +			      embedded_offset, addr, stream, recurse, options);
>        break;
>  
>      case TYPE_CODE_UNION:
> 

Update diff of what was pushed.  I fixed the formatting of the function comment.


From 1033c33c36c5f3d0b89d58aaaa374c76562583f3 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@ericsson.com>
Date: Thu, 9 Jul 2015 11:16:22 -0400
Subject: [PATCH] Factor out print_unpacked_pointer from c_val_print

Turn this code into a function, instead of a goto.

gdb/ChangeLog:

	* c-valprint.c (c_val_print): Factor out pointer printing code
	to ...
	(print_unpacked_pointer): ... this new function.
---
 gdb/ChangeLog    |   6 ++
 gdb/c-valprint.c | 205 ++++++++++++++++++++++++++++---------------------------
 2 files changed, 112 insertions(+), 99 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 443565a..1c8ec1d 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,11 @@
 2015-07-09  Simon Marchi  <simon.marchi@ericsson.com>

+	* c-valprint.c (c_val_print): Factor out pointer printing code
+	to ...
+	(print_unpacked_pointer): ... this new function.
+
+2015-07-09  Simon Marchi  <simon.marchi@ericsson.com>
+
 	* c-valprint.c (c_val_print): Remove an assignment to i and move
 	its declaration.

diff --git a/gdb/c-valprint.c b/gdb/c-valprint.c
index e8a2fc7..4648f17 100644
--- a/gdb/c-valprint.c
+++ b/gdb/c-valprint.c
@@ -127,6 +127,104 @@ static const struct generic_val_print_decorations c_decorations =
   "void"
 };

+/* Print a pointer based on the type of its target.
+
+   Arguments to this functions are roughly the same as those in c_val_print.
+   A difference is that ADDRESS is the address to print, with embedded_offset
+   already added.  UNRESOLVED_ELTTYPE and ELTTYPE represent the pointed type,
+   respectively before and after check_typedef.  */
+
+static void
+print_unpacked_pointer (struct type *type, struct type *elttype,
+			struct type *unresolved_elttype,
+			const gdb_byte *valaddr, int embedded_offset,
+			CORE_ADDR address, struct ui_file *stream, int recurse,
+			const struct value_print_options *options)
+{
+  int want_space = 0;
+  struct gdbarch *gdbarch = get_type_arch (type);
+
+  if (TYPE_CODE (elttype) == TYPE_CODE_FUNC)
+    {
+      /* Try to print what function it points to.  */
+      print_function_pointer_address (options, gdbarch, address, stream);
+      return;
+    }
+
+  if (options->symbol_print)
+    want_space = print_address_demangle (options, gdbarch, address, stream,
+					 demangle);
+  else if (options->addressprint)
+    {
+      fputs_filtered (paddress (gdbarch, address), stream);
+      want_space = 1;
+    }
+
+  /* For a pointer to a textual type, also print the string
+     pointed to, unless pointer is null.  */
+
+  if (c_textual_element_type (unresolved_elttype, options->format)
+      && address != 0)
+    {
+      if (want_space)
+	fputs_filtered (" ", stream);
+      val_print_string (unresolved_elttype, NULL, address, -1, stream, options);
+    }
+  else if (cp_is_vtbl_member (type))
+    {
+      /* Print vtbl's nicely.  */
+      CORE_ADDR vt_address = unpack_pointer (type, valaddr + embedded_offset);
+      struct bound_minimal_symbol msymbol =
+	lookup_minimal_symbol_by_pc (vt_address);
+
+      /* If 'symbol_print' is set, we did the work above.  */
+      if (!options->symbol_print
+	  && (msymbol.minsym != NULL)
+	  && (vt_address == BMSYMBOL_VALUE_ADDRESS (msymbol)))
+	{
+	  if (want_space)
+	    fputs_filtered (" ", stream);
+	  fputs_filtered (" <", stream);
+	  fputs_filtered (MSYMBOL_PRINT_NAME (msymbol.minsym), stream);
+	  fputs_filtered (">", stream);
+	  want_space = 1;
+	}
+
+      if (vt_address && options->vtblprint)
+	{
+	  struct value *vt_val;
+	  struct symbol *wsym = (struct symbol *) NULL;
+	  struct type *wtype;
+	  struct block *block = (struct block *) NULL;
+	  struct field_of_this_result is_this_fld;
+
+	  if (want_space)
+	    fputs_filtered (" ", stream);
+
+	  if (msymbol.minsym != NULL)
+	    wsym = lookup_symbol (MSYMBOL_LINKAGE_NAME(msymbol.minsym), block,
+				  VAR_DOMAIN, &is_this_fld);
+
+	  if (wsym)
+	    {
+	      wtype = SYMBOL_TYPE (wsym);
+	    }
+	  else
+	    {
+	      wtype = unresolved_elttype;
+	    }
+	  vt_val = value_at (wtype, vt_address);
+	  common_val_print (vt_val, stream, recurse + 1, options,
+			    current_language);
+	  if (options->prettyformat)
+	    {
+	      fprintf_filtered (stream, "\n");
+	      print_spaces_filtered (2 + 2 * recurse, stream);
+	    }
+	}
+    }
+}
+
 /* See val_print for a description of the various parameters of this
    function; they are identical.  */

@@ -237,10 +335,11 @@ c_val_print (struct type *type, const gdb_byte *valaddr,
 	    }
 	  break;
 	}
-      /* Array of unspecified length: treat like pointer to first
-	 elt.  */
-      addr = address + embedded_offset;
-      goto print_unpacked_pointer;
+      /* Array of unspecified length: treat like pointer to first elt.  */
+      print_unpacked_pointer (type, elttype, unresolved_elttype, valaddr,
+			      embedded_offset, address + embedded_offset,
+			      stream, recurse, options);
+      break;

     case TYPE_CODE_METHODPTR:
       cplus_print_method_ptr (valaddr + embedded_offset, type, stream);
@@ -267,101 +366,9 @@ c_val_print (struct type *type, const gdb_byte *valaddr,
 	}
       unresolved_elttype = TYPE_TARGET_TYPE (type);
       elttype = check_typedef (unresolved_elttype);
-	{
-	  int want_space;
-
-	  addr = unpack_pointer (type, valaddr + embedded_offset);
-	print_unpacked_pointer:
-
-	  want_space = 0;
-
-	  if (TYPE_CODE (elttype) == TYPE_CODE_FUNC)
-	    {
-	      /* Try to print what function it points to.  */
-	      print_function_pointer_address (options, gdbarch, addr, stream);
-	      return;
-	    }
-
-	  if (options->symbol_print)
-	    want_space = print_address_demangle (options, gdbarch, addr,
-						 stream, demangle);
-	  else if (options->addressprint)
-	    {
-	      fputs_filtered (paddress (gdbarch, addr), stream);
-	      want_space = 1;
-	    }
-
-	  /* For a pointer to a textual type, also print the string
-	     pointed to, unless pointer is null.  */
-
-	  if (c_textual_element_type (unresolved_elttype,
-				      options->format)
-	      && addr != 0)
-	    {
-	      if (want_space)
-		fputs_filtered (" ", stream);
-	      val_print_string (unresolved_elttype, NULL,
-				addr, -1,
-				stream, options);
-	    }
-	  else if (cp_is_vtbl_member (type))
-	    {
-	      /* Print vtbl's nicely.  */
-	      CORE_ADDR vt_address = unpack_pointer (type,
-						     valaddr
-						     + embedded_offset);
-	      struct bound_minimal_symbol msymbol =
-		lookup_minimal_symbol_by_pc (vt_address);
-
-	      /* If 'symbol_print' is set, we did the work above.  */
-	      if (!options->symbol_print
-		  && (msymbol.minsym != NULL)
-		  && (vt_address == BMSYMBOL_VALUE_ADDRESS (msymbol)))
-		{
-		  if (want_space)
-		    fputs_filtered (" ", stream);
-		  fputs_filtered (" <", stream);
-		  fputs_filtered (MSYMBOL_PRINT_NAME (msymbol.minsym), stream);
-		  fputs_filtered (">", stream);
-		  want_space = 1;
-		}
-
-	      if (vt_address && options->vtblprint)
-		{
-		  struct value *vt_val;
-		  struct symbol *wsym = (struct symbol *) NULL;
-		  struct type *wtype;
-		  struct block *block = (struct block *) NULL;
-		  struct field_of_this_result is_this_fld;
-
-		  if (want_space)
-		    fputs_filtered (" ", stream);
-
-		  if (msymbol.minsym != NULL)
-		    wsym = lookup_symbol (MSYMBOL_LINKAGE_NAME (msymbol.minsym),
-					  block, VAR_DOMAIN,
-					  &is_this_fld);
-
-		  if (wsym)
-		    {
-		      wtype = SYMBOL_TYPE (wsym);
-		    }
-		  else
-		    {
-		      wtype = unresolved_elttype;
-		    }
-		  vt_val = value_at (wtype, vt_address);
-		  common_val_print (vt_val, stream, recurse + 1,
-				    options, current_language);
-		  if (options->prettyformat)
-		    {
-		      fprintf_filtered (stream, "\n");
-		      print_spaces_filtered (2 + 2 * recurse, stream);
-		    }
-		}
-	    }
-	  return;
-	}
+      addr = unpack_pointer (type, valaddr + embedded_offset);
+      print_unpacked_pointer (type, elttype, unresolved_elttype, valaddr,
+			      embedded_offset, addr, stream, recurse, options);
       break;

     case TYPE_CODE_UNION:
-- 
2.1.4

Re: [PATCH 0/7] Split c_val_print

[Pedro Alves]

On 07/09/2015 04:28 PM, Simon Marchi wrote:

>>  - Per convention, make sure there's a line break after
>>    each variable declaration.
> 
> You mean, after each block of variable declarations, and not
> after each single variable declaration I suppose?

Whoops, yes, of course.  :-)

Thanks,
Pedro Alves

Re: [PATCH 4/7] Factor out pointer printing code from c_val_print

[Simon Marchi]

On 15-07-08 05:07 PM, Simon Marchi wrote:
> gdb/ChangeLog:
> 
> 	* c-valprint.c (c_val_print): Factor out pointer printing code
> 	to ...
> 	(c_val_print_ptr): ... this new function.
> ---
>  gdb/c-valprint.c | 62 +++++++++++++++++++++++++++++++++-----------------------
>  1 file changed, 37 insertions(+), 25 deletions(-)
> 
> diff --git a/gdb/c-valprint.c b/gdb/c-valprint.c
> index 0b95a7f..9f68815 100644
> --- a/gdb/c-valprint.c
> +++ b/gdb/c-valprint.c
> @@ -334,6 +334,41 @@ c_val_print_array (struct type *type, const gdb_byte *valaddr,
>      }
>  }
>  
> +/* c_val_print helper for TYPE_CODE_PTR.  */
> +
> +static void
> +c_val_print_ptr (struct type *type, const gdb_byte *valaddr,
> +		 int embedded_offset, struct ui_file *stream, int recurse,
> +		 const struct value *original_value,
> +		 const struct value_print_options *options)
> +{
> +  if (options->format && options->format != 's')
> +    {
> +      val_print_scalar_formatted (type, valaddr, embedded_offset,
> +				  original_value, options, 0, stream);
> +    }
> +  else if (options->vtblprint && cp_is_vtbl_ptr_type (type))
> +    {
> +      /* Print the unmangled name if desired.  */
> +      /* Print vtable entry - we only get here if we ARE using
> +	 -fvtable_thunks.  (Otherwise, look under
> +	 TYPE_CODE_STRUCT.)  */
> +      CORE_ADDR addr
> +	= extract_typed_address (valaddr + embedded_offset, type);
> +      struct gdbarch *gdbarch = get_type_arch (type);
> +
> +      print_function_pointer_address (options, gdbarch, addr, stream);
> +    }
> +  else
> +    {
> +      struct type *unresolved_elttype = TYPE_TARGET_TYPE (type);
> +      struct type *elttype = check_typedef (unresolved_elttype);
> +      CORE_ADDR addr = unpack_pointer (type, valaddr + embedded_offset);
> +      print_unpacked_pointer (type, elttype, unresolved_elttype, valaddr,
> +			      embedded_offset, addr, stream, recurse, options);
> +    }
> +}
> +
>  /* See val_print for a description of the various parameters of this
>     function; they are identical.  */
>  
> @@ -345,9 +380,7 @@ c_val_print (struct type *type, const gdb_byte *valaddr,
>  	     const struct value_print_options *options)
>  {
>    struct gdbarch *gdbarch = get_type_arch (type);
> -  struct type *elttype, *unresolved_elttype;
>    struct type *unresolved_type = type;
> -  CORE_ADDR addr;
>  
>    CHECK_TYPEDEF (type);
>    switch (TYPE_CODE (type))
> @@ -362,29 +395,8 @@ c_val_print (struct type *type, const gdb_byte *valaddr,
>        break;
>  
>      case TYPE_CODE_PTR:
> -      if (options->format && options->format != 's')
> -	{
> -	  val_print_scalar_formatted (type, valaddr, embedded_offset,
> -				      original_value, options, 0, stream);
> -	  break;
> -	}
> -      if (options->vtblprint && cp_is_vtbl_ptr_type (type))
> -	{
> -	  /* Print the unmangled name if desired.  */
> -	  /* Print vtable entry - we only get here if we ARE using
> -	     -fvtable_thunks.  (Otherwise, look under
> -	     TYPE_CODE_STRUCT.)  */
> -	  CORE_ADDR addr
> -	    = extract_typed_address (valaddr + embedded_offset, type);
> -
> -	  print_function_pointer_address (options, gdbarch, addr, stream);
> -	  break;
> -	}
> -      unresolved_elttype = TYPE_TARGET_TYPE (type);
> -      elttype = check_typedef (unresolved_elttype);
> -      addr = unpack_pointer (type, valaddr + embedded_offset);
> -      print_unpacked_pointer (type, elttype, unresolved_elttype, valaddr,
> -			      embedded_offset, addr, stream, recurse, options);
> +      c_val_print_ptr (type, valaddr, embedded_offset, stream, recurse,
> +		       original_value, options);
>        break;
>  
>      case TYPE_CODE_UNION:
> 

Updated diff of what was pushed, I added a new line before a variable declaration.


From 1c67f0326351d9c521692ceca92f93cb0de9bf2e Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@ericsson.com>
Date: Thu, 9 Jul 2015 11:18:12 -0400
Subject: [PATCH] Factor out pointer printing code from c_val_print

gdb/ChangeLog:

	* c-valprint.c (c_val_print): Factor out pointer printing code
	to ...
	(c_val_print_ptr): ... this new function.
---
 gdb/ChangeLog    |  6 ++++++
 gdb/c-valprint.c | 63 ++++++++++++++++++++++++++++++++++----------------------
 2 files changed, 44 insertions(+), 25 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index f2ac3a6..54f7374 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,11 @@
 2015-07-09  Simon Marchi  <simon.marchi@ericsson.com>

+	* c-valprint.c (c_val_print): Factor out pointer printing code
+	to ...
+	(c_val_print_ptr): ... this new function.
+
+2015-07-09  Simon Marchi  <simon.marchi@ericsson.com>
+
 	* c-valprint.c (c_valprint): Factor our array printing code from
 	c_val_print to ...
 	(c_val_print_array): ... this new function.
diff --git a/gdb/c-valprint.c b/gdb/c-valprint.c
index 9f9d999..4853575 100644
--- a/gdb/c-valprint.c
+++ b/gdb/c-valprint.c
@@ -334,6 +334,42 @@ c_val_print_array (struct type *type, const gdb_byte *valaddr,
     }
 }

+/* c_val_print helper for TYPE_CODE_PTR.  */
+
+static void
+c_val_print_ptr (struct type *type, const gdb_byte *valaddr,
+		 int embedded_offset, struct ui_file *stream, int recurse,
+		 const struct value *original_value,
+		 const struct value_print_options *options)
+{
+  if (options->format && options->format != 's')
+    {
+      val_print_scalar_formatted (type, valaddr, embedded_offset,
+				  original_value, options, 0, stream);
+    }
+  else if (options->vtblprint && cp_is_vtbl_ptr_type (type))
+    {
+      /* Print the unmangled name if desired.  */
+      /* Print vtable entry - we only get here if we ARE using
+	 -fvtable_thunks.  (Otherwise, look under
+	 TYPE_CODE_STRUCT.)  */
+      CORE_ADDR addr
+	= extract_typed_address (valaddr + embedded_offset, type);
+      struct gdbarch *gdbarch = get_type_arch (type);
+
+      print_function_pointer_address (options, gdbarch, addr, stream);
+    }
+  else
+    {
+      struct type *unresolved_elttype = TYPE_TARGET_TYPE (type);
+      struct type *elttype = check_typedef (unresolved_elttype);
+      CORE_ADDR addr = unpack_pointer (type, valaddr + embedded_offset);
+
+      print_unpacked_pointer (type, elttype, unresolved_elttype, valaddr,
+			      embedded_offset, addr, stream, recurse, options);
+    }
+}
+
 /* See val_print for a description of the various parameters of this
    function; they are identical.  */

@@ -345,9 +381,7 @@ c_val_print (struct type *type, const gdb_byte *valaddr,
 	     const struct value_print_options *options)
 {
   struct gdbarch *gdbarch = get_type_arch (type);
-  struct type *elttype, *unresolved_elttype;
   struct type *unresolved_type = type;
-  CORE_ADDR addr;

   CHECK_TYPEDEF (type);
   switch (TYPE_CODE (type))
@@ -362,29 +396,8 @@ c_val_print (struct type *type, const gdb_byte *valaddr,
       break;

     case TYPE_CODE_PTR:
-      if (options->format && options->format != 's')
-	{
-	  val_print_scalar_formatted (type, valaddr, embedded_offset,
-				      original_value, options, 0, stream);
-	  break;
-	}
-      if (options->vtblprint && cp_is_vtbl_ptr_type (type))
-	{
-	  /* Print the unmangled name if desired.  */
-	  /* Print vtable entry - we only get here if we ARE using
-	     -fvtable_thunks.  (Otherwise, look under
-	     TYPE_CODE_STRUCT.)  */
-	  CORE_ADDR addr
-	    = extract_typed_address (valaddr + embedded_offset, type);
-
-	  print_function_pointer_address (options, gdbarch, addr, stream);
-	  break;
-	}
-      unresolved_elttype = TYPE_TARGET_TYPE (type);
-      elttype = check_typedef (unresolved_elttype);
-      addr = unpack_pointer (type, valaddr + embedded_offset);
-      print_unpacked_pointer (type, elttype, unresolved_elttype, valaddr,
-			      embedded_offset, addr, stream, recurse, options);
+      c_val_print_ptr (type, valaddr, embedded_offset, stream, recurse,
+		       original_value, options);
       break;

     case TYPE_CODE_UNION:
-- 
2.1.4

Re: [PATCH 0/7] Split c_val_print

[Sergio Durigan Junior]

On Wednesday, July 08 2015, Simon Marchi wrote:

> I think that c_val_print deserves to be split up in smaller, more
> manageable chunks.  This series does that, by simply factoring out each
> case of the big switch.  In some cases, a bit of modifications were
> necessary where fallthrough between cases or goto were used, but
> otherwise the code stays the same.

Hey Simon,

Could you please take a look at the BuildBot results for your series?
I'm seeing some strange errors there, not sure if they were caused by
these patches or not.

Thanks,

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/