[PATCH] Fix coding style in gcore.c

[Mark Kettenis <kettenis at chello dot nl>]

The attached patch cleans up the coding style in gcore.c.  This is
mostly pretty-printing.  However I converted some tests that were
checking for something that "can't happen" into gdb_asserts.

Committed.

Index: ChangeLog
from  Mark Kettenis  <m.kettenis@osp.nl>

	* gcore.c: Reorder include files in alphabetical order.  Include
	"gdb_assert.h".  Various coding style fixes.
	(derive_stack_segment, derive_heap_segment): Replace check for
	non-null BOTTOM and TOP with gdb_assert.
	(derive_heap_segment): Replace check for successful creation of
	ZERO with gdb_assert.
	(make_mem_sec): Use bfd_section_lma to set OSEC->lma.

Index: gcore.c
===================================================================
RCS file: /cvs/src/src/gdb/gcore.c,v
retrieving revision 1.10
diff -u -p -r1.10 gcore.c
--- gcore.c 21 Apr 2003 16:48:37 -0000 1.10
+++ gcore.c 4 Sep 2003 20:45:25 -0000
@@ -20,21 +20,23 @@
    Boston, MA 02111-1307, USA.  */
 
 #include "defs.h"
-#include "cli/cli-decode.h"
+#include "elf-bfd.h"
+#include "infcall.h"
 #include "inferior.h"
 #include "gdbcore.h"
-#include "elf-bfd.h"
-#include "symfile.h"
 #include "objfiles.h"
-#include "infcall.h"
+#include "symfile.h"
+
+#include "cli/cli-decode.h"
 
-static char                  *default_gcore_target (void);
-static enum bfd_architecture  default_gcore_arch (void);
-static unsigned long          default_gcore_mach (void);
-static int                    gcore_memory_sections (bfd *);
+#include "gdb_assert.h"
 
-/* Function: gcore_command
-   Generate a core file from the inferior process.  */
+static char *default_gcore_target (void);
+static enum bfd_architecture default_gcore_arch (void);
+static unsigned long default_gcore_mach (void);
+static int gcore_memory_sections (bfd *);
+
+/* Generate a core file from the inferior process.  */
 
 static void
 gcore_command (char *args, int from_tty)
@@ -47,7 +49,7 @@ gcore_command (char *args, int from_tty)
   int note_size = 0;
 
   /* No use generating a corefile without a target process.  */
-  if (!(target_has_execution))
+  if (!target_has_execution)
     noprocess ();
 
   if (args && *args)
@@ -63,25 +65,25 @@ gcore_command (char *args, int from_tty)
     fprintf_filtered (gdb_stdout, 
 		      "Opening corefile '%s' for output.\n", corefilename);
 
-  /* Open the output file. */
-  if (!(obfd = bfd_openw (corefilename, default_gcore_target ())))
-    {
-      error ("Failed to open '%s' for output.", corefilename);
-    }
+  /* Open the output file.  */
+  obfd = bfd_openw (corefilename, default_gcore_target ());
+  if (!obfd)
+    error ("Failed to open '%s' for output.", corefilename);
 
-  /* Need a cleanup that will close the file (FIXME: delete it?). */
+  /* Need a cleanup that will close the file (FIXME: delete it?).  */
   old_chain = make_cleanup_bfd_close (obfd);
 
   bfd_set_format (obfd, bfd_core);
   bfd_set_arch_mach (obfd, default_gcore_arch (), default_gcore_mach ());
 
-  /* An external target method must build the notes section. */
-  note_data = (char *) target_make_corefile_notes (obfd, &note_size);
+  /* An external target method must build the notes section.  */
+  note_data = target_make_corefile_notes (obfd, &note_size);
 
-  /* Create the note section. */
+  /* Create the note section.  */
   if (note_data != NULL && note_size != 0)
     {
-      if ((note_sec = bfd_make_section_anyway (obfd, "note0")) == NULL)
+      note_sec = bfd_make_section_anyway (obfd, "note0");
+      if (note_sec == NULL)
 	error ("Failed to create 'note' section for corefile: %s", 
 	       bfd_errmsg (bfd_get_error ()));
 
@@ -92,25 +94,21 @@ gcore_command (char *args, int from_tty)
       bfd_set_section_size (obfd, note_sec, note_size);
     }
 
-  /* Now create the memory/load sections. */
+  /* Now create the memory/load sections.  */
   if (gcore_memory_sections (obfd) == 0)
     error ("gcore: failed to get corefile memory sections from target.");
 
-  /* Write out the contents of the note section. */
+  /* Write out the contents of the note section.  */
   if (note_data != NULL && note_size != 0)
     {
       if (!bfd_set_section_contents (obfd, note_sec, note_data, 0, note_size))
-	{
-	  warning ("writing note section (%s)", 
-		   bfd_errmsg (bfd_get_error ()));
-	}
+	warning ("writing note section (%s)", bfd_errmsg (bfd_get_error ()));
     }
 
-  /* Succeeded. */
-  fprintf_filtered (gdb_stdout, 
-		    "Saved corefile %s\n", corefilename);
+  /* Succeeded.  */
+  fprintf_filtered (gdb_stdout, "Saved corefile %s\n", corefilename);
 
-  /* Clean-ups will close the output file and free malloc memory. */
+  /* Clean-ups will close the output file and free malloc memory.  */
   do_cleanups (old_chain);
   return;
 }
@@ -118,11 +116,11 @@ gcore_command (char *args, int from_tty)
 static unsigned long
 default_gcore_mach (void)
 {
-#if 1	/* See if this even matters... */
+#if 1	/* See if this even matters...  */
   return 0;
 #else
 #ifdef TARGET_ARCHITECTURE
-  const struct bfd_arch_info * bfdarch = TARGET_ARCHITECTURE;
+  const struct bfd_arch_info *bfdarch = TARGET_ARCHITECTURE;
 
   if (bfdarch != NULL)
     return bfdarch->mach;
@@ -152,65 +150,64 @@ default_gcore_arch (void)
 static char *
 default_gcore_target (void)
 {
-  /* FIXME -- this may only work for ELF targets.  */
+  /* FIXME: This may only work for ELF targets.  */
   if (exec_bfd == NULL)
     return NULL;
   else
     return bfd_get_target (exec_bfd);
 }
 
-/* Function: derive_stack_segment
-
-   Derive a reasonable stack segment by unwinding the target stack. 
-   
-   Returns 0 for failure, 1 for success.  */
+/* Derive a reasonable stack segment by unwinding the target stack,
+   and store its limits in *BOTTOM and *TOP.  Return non-zero if
+   successful.  */
 
 static int 
 derive_stack_segment (bfd_vma *bottom, bfd_vma *top)
 {
-  bfd_vma tmp_vma;
   struct frame_info *fi, *tmp_fi;
 
-  if (bottom == NULL || top == NULL)
-    return 0;	/* Paranoia. */
+  gdb_assert (bottom);
+  gdb_assert (top);
 
+  /* Can't succeed without stack and registers.  */
   if (!target_has_stack || !target_has_registers)
-    return 0;	/* Can't succeed without stack and registers. */
+    return 0;
 
-  if ((fi = get_current_frame ()) == NULL)
-    return 0;	/* Can't succeed without current frame. */
+  /* Can't succeed without current frame.  */
+  fi = get_current_frame ();
+  if (fi == NULL)
+    return 0;
 
-  /* Save frame pointer of TOS frame. */
+  /* Save frame pointer of TOS frame.  */
   *top = get_frame_base (fi);
-  /* If current stack pointer is more "inner", use that instead. */
+  /* If current stack pointer is more "inner", use that instead.  */
   if (INNER_THAN (read_sp (), *top))
     *top = read_sp ();
 
-  /* Find prev-most frame. */
+  /* Find prev-most frame.  */
   while ((tmp_fi = get_prev_frame (fi)) != NULL)
     fi = tmp_fi;
 
-  /* Save frame pointer of prev-most frame. */
+  /* Save frame pointer of prev-most frame.  */
   *bottom = get_frame_base (fi);
 
-  /* Now canonicalize their order, so that 'bottom' is a lower address
-   (as opposed to a lower stack frame). */
+  /* Now canonicalize their order, so that BOTTOM is a lower address
+     (as opposed to a lower stack frame).  */
   if (*bottom > *top)
     {
+      bfd_vma tmp_vma;
+
       tmp_vma = *top;
       *top = *bottom;
       *bottom = tmp_vma;
     }
 
-  return 1;	/* success */
+  return 1;
 }
 
-/* Function: derive_heap_segment
-
-   Derive a reasonable heap segment by looking at sbrk and
-   the static data sections.
-   
-   Returns 0 for failure, 1 for success.  */
+/* Derive a reasonable heap segment for ABFD by looking at sbrk and
+   the static data sections.  Store its limits in *BOTTOM and *TOP.
+   Return non-zero if successful.  */
 
 static int 
 derive_heap_segment (bfd *abfd, bfd_vma *bottom, bfd_vma *top)
@@ -222,23 +219,29 @@ derive_heap_segment (bfd *abfd, bfd_vma 
   bfd_vma sec_vaddr;
   asection *sec;
 
-  if (bottom == NULL || top == NULL)
-    return 0;		/* Paranoia. */
+  gdb_assert (bottom);
+  gdb_assert (top);
 
+  /* This function depends on being able to call a function in the
+     inferior.  */
   if (!target_has_execution)
-    return 0;		/* This function depends on being able
-			   to call a function in the inferior.  */
+    return 0;
+
+  /* The following code assumes that the link map is arranged as
+     follows (low to high addresses):
 
-  /* Assumption: link map is arranged as follows (low to high addresses):
-     text sections
-     data sections (including bss)
-     heap
-  */
+     ---------------------------------
+     | text sections                 |
+     ---------------------------------
+     | data sections (including bss) |
+     ---------------------------------
+     | heap                          |
+     --------------------------------- */
 
   for (sec = abfd->sections; sec; sec = sec->next)
     {
-      if (bfd_get_section_flags (abfd, sec) & SEC_DATA ||
-	  strcmp (".bss", bfd_section_name (abfd, sec)) == 0)
+      if (bfd_get_section_flags (abfd, sec) & SEC_DATA
+	  || strcmp (".bss", bfd_section_name (abfd, sec)) == 0)
 	{
 	  sec_vaddr = bfd_get_section_vma (abfd, sec);
 	  sec_size = bfd_get_section_size_before_reloc (sec);
@@ -246,35 +249,40 @@ derive_heap_segment (bfd *abfd, bfd_vma 
 	    top_of_data_memory = sec_vaddr + sec_size;
 	}
     }
+
   /* Now get the top-of-heap by calling sbrk in the inferior.  */
   if (lookup_minimal_symbol ("sbrk", NULL, NULL) != NULL)
     {
-      if ((sbrk = find_function_in_inferior ("sbrk")) == NULL)
+      sbrk = find_function_in_inferior ("sbrk");
+      if (sbrk == NULL)
 	return 0;
     }
   else if (lookup_minimal_symbol ("_sbrk", NULL, NULL) != NULL)
     {
-      if ((sbrk = find_function_in_inferior ("_sbrk")) == NULL)
+      sbrk = find_function_in_inferior ("_sbrk");
+      if (sbrk == NULL)
 	return 0;
     }
   else
     return 0;
 
-  if ((zero = value_from_longest (builtin_type_int, (LONGEST) 0)) == NULL)
-    return 0;
-  if ((sbrk = call_function_by_hand (sbrk, 1, &zero)) == NULL)
+  zero = value_from_longest (builtin_type_int, 0);
+  gdb_assert (zero);
+  sbrk = call_function_by_hand (sbrk, 1, &zero);
+  if (sbrk == NULL)
     return 0;
   top_of_heap = value_as_long (sbrk);
 
-  /* Return results. */
+  /* Return results.  */
   if (top_of_heap > top_of_data_memory)
     {
       *bottom = top_of_data_memory;
       *top = top_of_heap;
-      return 1;	/* success */
+      return 1;
     }
-  else
-    return 0;	/* No additional heap space needs to be saved. */
+
+  /* No additional heap space needs to be saved.  */
+  return 0;
 }
 
 /* ARGSUSED */
@@ -296,20 +304,17 @@ make_output_phdrs (bfd *obfd, asection *
   if (bfd_get_section_flags (obfd, osec) & SEC_CODE)
     p_flags |= PF_X;	/* Segment is executable.  */
 
-  bfd_record_phdr (obfd, p_type, 1, p_flags, 0, 0, 
-		   0, 0, 1, &osec);
+  bfd_record_phdr (obfd, p_type, 1, p_flags, 0, 0, 0, 0, 1, &osec);
 }
 
 static asection *
-make_mem_sec (bfd *obfd, 
-	      bfd_vma addr, 
-	      bfd_size_type size, 
-	      unsigned int flags, 
-	      unsigned int alignment)
+make_mem_sec (bfd *obfd, bfd_vma addr, bfd_size_type size,
+	      unsigned int flags, unsigned int alignment)
 {
   asection *osec;
 
-  if ((osec = bfd_make_section_anyway (obfd, "load")) == NULL)
+  osec = bfd_make_section_anyway (obfd, "load");
+  if (osec == NULL)
     {
       warning ("Couldn't make gcore segment: %s",
 	       bfd_errmsg (bfd_get_error ()));
@@ -318,59 +323,52 @@ make_mem_sec (bfd *obfd, 
 
   if (info_verbose)
     {
-      fprintf_filtered (gdb_stdout, 
-			"Save segment, %lld bytes at 0x%s\n",
+      fprintf_filtered (gdb_stdout, "Save segment, %lld bytes at 0x%s\n",
 			(long long) size, paddr_nz (addr));
     }
 
   bfd_set_section_size (obfd, osec, size);
   bfd_set_section_vma (obfd, osec, addr);
-  osec->lma = 0;	/* FIXME: there should be a macro for this! */
+  bfd_section_lma (obfd, osec) = 0; /* ??? bfd_set_section_lma?  */
   bfd_set_section_alignment (obfd, osec, alignment);
-  bfd_set_section_flags (obfd, osec, 
+  bfd_set_section_flags (obfd, osec,
 			 flags | SEC_LOAD | SEC_ALLOC | SEC_HAS_CONTENTS);
   return osec;
 }
 
 static int
-gcore_create_callback (CORE_ADDR vaddr, 
-		       unsigned long size,
-		       int read, int write, int exec, 
-		       void *data)
+gcore_create_callback (CORE_ADDR vaddr, unsigned long size,
+		       int read, int write, int exec, void *data)
 {
   flagword flags = 0;
 
   if (write == 0)
     {
       flags |= SEC_READONLY;
-      /* Set size == zero for readonly sections. */
+      /* Mark readonly sections as zero-sized, such that we can avoid
+         copying their contents.  */
       size = 0;
     }
+
   if (exec)
-    {
-      flags |= SEC_CODE;
-    }
+    flags |= SEC_CODE;
   else
-    {
-      flags |= SEC_DATA;
-    }
+    flags |= SEC_DATA;
 
-  return ((make_mem_sec ((bfd *) data, vaddr, size, flags, 0)) == NULL);
+  return ((make_mem_sec (data, vaddr, size, flags, 0)) == NULL);
 }
 
 static int
-objfile_find_memory_regions (int (*func) (CORE_ADDR, 
-					  unsigned long,
-					  int, int, int,
-					   void *), 
+objfile_find_memory_regions (int (*func) (CORE_ADDR, unsigned long,
+					  int, int, int, void *),
 			     void *obfd)
 {
-  /* Use objfile data to create memory sections. */
+  /* Use objfile data to create memory sections.  */
   struct objfile *objfile;
   struct obj_section *objsec;
   bfd_vma temp_bottom, temp_top;
 
-  /* Call callback function for each objfile section. */
+  /* Call callback function for each objfile section.  */
   ALL_OBJSECTIONS (objfile, objsec)
     {
       bfd *ibfd = objfile->obfd;
@@ -383,33 +381,32 @@ objfile_find_memory_regions (int (*func)
 	  int size = bfd_section_size (ibfd, isec);
 	  int ret;
 
-	  if ((ret = (*func) (objsec->addr, 
-			      bfd_section_size (ibfd, isec), 
-			      1, /* All sections will be readable.  */
-			      (flags & SEC_READONLY) == 0, /* writable */
-			      (flags & SEC_CODE) != 0, /* executable */
-			      obfd)) != 0)
+	  ret = (*func) (objsec->addr, bfd_section_size (ibfd, isec), 
+			 1, /* All sections will be readable.  */
+			 (flags & SEC_READONLY) == 0, /* Writable.  */
+			 (flags & SEC_CODE) != 0, /* Executable.  */
+			 obfd);
+	  if (ret != 0)
 	    return ret;
 	}
     }
 
-  /* Make a stack segment. */
+  /* Make a stack segment.  */
   if (derive_stack_segment (&temp_bottom, &temp_top))
-    (*func) (temp_bottom, 
-	     temp_top - temp_bottom, 
-	     1, /* Stack section will be readable */
-	     1, /* Stack section will be writable */
-	     0, /* Stack section will not be executable */
+    (*func) (temp_bottom, temp_top - temp_bottom, 
+	     1, /* Stack section will be readable.  */
+	     1, /* Stack section will be writable.  */
+	     0, /* Stack section will not be executable.  */
 	     obfd);
 
   /* Make a heap segment. */
   if (derive_heap_segment (exec_bfd, &temp_bottom, &temp_top))
-    (*func) (temp_bottom, 
-	     temp_top - temp_bottom, 
-	     1, /* Heap section will be readable */
-	     1, /* Heap section will be writable */
-	     0, /* Heap section will not be executable */
+    (*func) (temp_bottom, temp_top - temp_bottom,
+	     1, /* Heap section will be readable.  */
+	     1, /* Heap section will be writable.  */
+	     0, /* Heap section will not be executable.  */
 	     obfd);
+
   return 0;
 }
 
@@ -420,13 +417,18 @@ gcore_copy_callback (bfd *obfd, asection
   struct cleanup *old_chain = NULL;
   void *memhunk;
 
+  /* Read-only sections are marked as zero-size.  We don't have to
+     copy their contents.  */
   if (size == 0)
-    return;	/* Read-only sections are marked as zero-size.
-		   We don't have to copy their contents. */
+    return;
+
+  /* Only interested in "load" sections.  */
   if (strncmp ("load", bfd_section_name (obfd, osec), 4) != 0)
-    return;	/* Only interested in "load" sections. */
+    return;
 
-  if ((memhunk = xmalloc (size)) == NULL)
+  memhunk = xmalloc (size);
+  /* ??? This is crap since xmalloc should never return NULL.  */
+  if (memhunk == NULL)
     error ("Not enough memory to create corefile.");
   old_chain = make_cleanup (xfree, memhunk);
 
@@ -438,29 +440,30 @@ gcore_copy_callback (bfd *obfd, asection
     warning ("Failed to write corefile contents (%s).", 
 	     bfd_errmsg (bfd_get_error ()));
 
-  do_cleanups (old_chain);	/* frees the xmalloc buffer */
+  do_cleanups (old_chain);	/* Frees MEMHUNK.  */
 }
 
 static int
 gcore_memory_sections (bfd *obfd)
 {
   if (target_find_memory_regions (gcore_create_callback, obfd) != 0)
-    return 0;	/* FIXME error return/msg? */
+    return 0;			/* FIXME: error return/msg?  */
 
-  /* Record phdrs for section-to-segment mapping. */
+  /* Record phdrs for section-to-segment mapping.  */
   bfd_map_over_sections (obfd, make_output_phdrs, NULL);
 
-  /* Copy memory region contents. */
+  /* Copy memory region contents.  */
   bfd_map_over_sections (obfd, gcore_copy_callback, NULL);
 
-  return 1;	/* success */
+  return 1;
 }
 
 void
 _initialize_gcore (void)
 {
   add_com ("generate-core-file", class_files, gcore_command,
-	   "Save a core file with the current state of the debugged process.\n\
+	   "\
+Save a core file with the current state of the debugged process.\n\
 Argument is optional filename.  Default filename is 'core.<process_id>'.");
 
   add_com_alias ("gcore", "generate-core-file", class_files, 1);