[patch] Code cleanup: bpstat_alloc calling semantics standardization

[patch] Code cleanup: bpstat_alloc calling semantics standardization

[Jan Kratochvil]

Hi,

this is just for the second patch I am going to send in a minute.  I would not
patch it standalone but I could not manage to code anything on top of it.

No regressions on {x86_64,x86_64-m32,i686}-fedora14snapshot-linux-gnu.
(Tested together with the second patch.)


Thanks,
Jan


2010-08-11  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Code cleanup.
	* breakpoint.c (bpstat_alloc): Remove unused prototype.
	(bpstat_alloc): Change parameters cbs to bs_link_pointer.  Adjust the
	code.
	(bpstat_stop_status): Change root_bs into bs_head and bs_link.  Adjust
	calls of bpstat_alloc.  Remove explicit bs chain termination.

--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -135,8 +135,6 @@ static void watchpoints_info (char *, int);
 
 static int breakpoint_1 (int, int, int (*) (const struct breakpoint *));
 
-static bpstat bpstat_alloc (const struct bp_location *, bpstat);
-
 static int breakpoint_cond_eval (void *);
 
 static void cleanup_executing_breakpoints (void *);
@@ -3442,15 +3440,17 @@ breakpoint_cond_eval (void *exp)
   return i;
 }
 
-/* Allocate a new bpstat and chain it to the current one.  */
+/* Allocate a new bpstat.  Link it to the FIFO list by BS_LINK_POINTER.  */
 
 static bpstat
-bpstat_alloc (const struct bp_location *bl, bpstat cbs /* Current "bs" value */ )
+bpstat_alloc (const struct bp_location *bl, bpstat **bs_link_pointer)
 {
   bpstat bs;
 
   bs = (bpstat) xmalloc (sizeof (*bs));
-  cbs->next = bs;
+  bs->next = NULL;
+  **bs_link_pointer = bs;
+  *bs_link_pointer = &bs->next;
   bs->breakpoint_at = bl;
   /* If the condition is false, etc., don't do the commands.  */
   bs->commands = NULL;
@@ -4027,10 +4027,10 @@ bpstat_stop_status (struct address_space *aspace,
   struct breakpoint *b = NULL;
   struct bp_location *bl;
   struct bp_location *loc;
-  /* Root of the chain of bpstat's */
-  struct bpstats root_bs[1];
+  /* First item of allocated bpstat's.  */
+  bpstat bs_head = NULL, *bs_link = &bs_head;
   /* Pointer to the last thing in the chain currently.  */
-  bpstat bs = root_bs;
+  bpstat bs;
   int ix;
   int need_remove_insert;
 
@@ -4061,7 +4061,7 @@ bpstat_stop_status (struct address_space *aspace,
 
 	  /* Come here if it's a watchpoint, or if the break address matches */
 
-	  bs = bpstat_alloc (bl, bs);	/* Alloc a bpstat to explain stop */
+	  bs = bpstat_alloc (bl, &bs_link);	/* Alloc a bpstat to explain stop */
 
 	  /* Assume we stop.  Should we find watchpoint that is not actually
 	     triggered, or if condition of breakpoint is false, we'll reset
@@ -4119,7 +4119,7 @@ bpstat_stop_status (struct address_space *aspace,
       if (breakpoint_address_match (loc->pspace->aspace, loc->address,
 				    aspace, bp_addr))
 	{
-	  bs = bpstat_alloc (loc, bs);
+	  bs = bpstat_alloc (loc, &bs_link);
 	  /* For hits of moribund locations, we should just proceed.  */
 	  bs->stop = 0;
 	  bs->print = 0;
@@ -4127,15 +4127,13 @@ bpstat_stop_status (struct address_space *aspace,
 	}
     }
 
-  bs->next = NULL;		/* Terminate the chain */
-
   /* If we aren't stopping, the value of some hardware watchpoint may
      not have changed, but the intermediate memory locations we are
      watching may have.  Don't bother if we're stopping; this will get
      done later.  */
   need_remove_insert = 0;
-  if (! bpstat_causes_stop (root_bs->next))
-    for (bs = root_bs->next; bs != NULL; bs = bs->next)
+  if (! bpstat_causes_stop (bs_head))
+    for (bs = bs_head; bs != NULL; bs = bs->next)
       if (!bs->stop
 	  && bs->breakpoint_at->owner
 	  && is_hardware_watchpoint (bs->breakpoint_at->owner))
@@ -4150,7 +4148,7 @@ bpstat_stop_status (struct address_space *aspace,
   if (need_remove_insert)
     update_global_location_list (1);
 
-  return root_bs->next;
+  return bs_head;
 }
 
 static void

Re: [patch] Code cleanup: bpstat_alloc calling semantics standardization

[Tom Tromey]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> this is just for the second patch I am going to send in a minute.
Jan> I would not patch it standalone but I could not manage to code
Jan> anything on top of it.

Just to be clear, I assume this is obsoleted by Pedro's other fix for PR
11371.

Tom

Re: [patch] Code cleanup: bpstat_alloc calling semantics standardization

[Jan Kratochvil]

On Thu, 19 Aug 2010 00:48:42 +0200, Tom Tromey wrote:
> >>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
> 
> Jan> this is just for the second patch I am going to send in a minute.
> Jan> I would not patch it standalone but I could not manage to code
> Jan> anything on top of it.
> 
> Just to be clear, I assume this is obsoleted by Pedro's other fix for PR
> 11371.

It is no longer a prerequisite for the PR 11371 fix.

Still it is a valid code cleanup.  Which is not much of worth.

Updated the patch.

No regressions on {x86_64,x86_64-m32,i686}-fedora14snapshot-linux-gnu.


Thanks,
Jan


2010-08-19  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Code cleanup.
	* breakpoint.c (bpstat_alloc): Remove unused prototype.
	(bpstat_alloc): Change parameters cbs to bs_link_pointer.  Adjust the
	code.
	(bpstat_stop_status): Change root_bs into bs_head and bs_link.  Adjust
	calls of bpstat_alloc.  Remove explicit bs chain termination.

--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -133,8 +133,6 @@ static void watchpoints_info (char *, int);
 
 static int breakpoint_1 (int, int, int (*) (const struct breakpoint *));
 
-static bpstat bpstat_alloc (struct bp_location *, bpstat);
-
 static int breakpoint_cond_eval (void *);
 
 static void cleanup_executing_breakpoints (void *);
@@ -3436,15 +3434,17 @@ breakpoint_cond_eval (void *exp)
   return i;
 }
 
-/* Allocate a new bpstat and chain it to the current one.  */
+/* Allocate a new bpstat.  Link it to the FIFO list by BS_LINK_POINTER.  */
 
 static bpstat
-bpstat_alloc (struct bp_location *bl, bpstat cbs /* Current "bs" value */ )
+bpstat_alloc (struct bp_location *bl, bpstat **bs_link_pointer)
 {
   bpstat bs;
 
   bs = (bpstat) xmalloc (sizeof (*bs));
-  cbs->next = bs;
+  bs->next = NULL;
+  **bs_link_pointer = bs;
+  *bs_link_pointer = &bs->next;
   bs->breakpoint_at = bl->owner;
   bs->bp_location_at = bl;
   incref_bp_location (bl);
@@ -4016,10 +4016,10 @@ bpstat_stop_status (struct address_space *aspace,
   struct breakpoint *b = NULL;
   struct bp_location *bl;
   struct bp_location *loc;
-  /* Root of the chain of bpstat's */
-  struct bpstats root_bs[1];
+  /* First item of allocated bpstat's.  */
+  bpstat bs_head = NULL, *bs_link = &bs_head;
   /* Pointer to the last thing in the chain currently.  */
-  bpstat bs = root_bs;
+  bpstat bs;
   int ix;
   int need_remove_insert;
   int removed_any;
@@ -4054,7 +4054,7 @@ bpstat_stop_status (struct address_space *aspace,
 
 	  /* Come here if it's a watchpoint, or if the break address matches */
 
-	  bs = bpstat_alloc (bl, bs);	/* Alloc a bpstat to explain stop */
+	  bs = bpstat_alloc (bl, &bs_link);	/* Alloc a bpstat to explain stop */
 
 	  /* Assume we stop.  Should we find a watchpoint that is not
 	     actually triggered, or if the condition of the breakpoint
@@ -4076,7 +4076,7 @@ bpstat_stop_status (struct address_space *aspace,
       if (breakpoint_address_match (loc->pspace->aspace, loc->address,
 				    aspace, bp_addr))
 	{
-	  bs = bpstat_alloc (loc, bs);
+	  bs = bpstat_alloc (loc, &bs_link);
 	  /* For hits of moribund locations, we should just proceed.  */
 	  bs->stop = 0;
 	  bs->print = 0;
@@ -4084,16 +4084,13 @@ bpstat_stop_status (struct address_space *aspace,
 	}
     }
 
-  /* Terminate the chain.  */
-  bs->next = NULL;
-
   /* Now go through the locations that caused the target to stop, and
      check whether we're interested in reporting this stop to higher
      layers, or whether we should resume the target transparently.  */
 
   removed_any = 0;
 
-  for (bs = root_bs->next; bs != NULL; bs = bs->next)
+  for (bs = bs_head; bs != NULL; bs = bs->next)
     {
       if (!bs->stop)
 	continue;
@@ -4149,8 +4146,8 @@ bpstat_stop_status (struct address_space *aspace,
      watching may have.  Don't bother if we're stopping; this will get
      done later.  */
   need_remove_insert = 0;
-  if (! bpstat_causes_stop (root_bs->next))
-    for (bs = root_bs->next; bs != NULL; bs = bs->next)
+  if (! bpstat_causes_stop (bs_head))
+    for (bs = bs_head; bs != NULL; bs = bs->next)
       if (!bs->stop
 	  && bs->breakpoint_at
 	  && is_hardware_watchpoint (bs->breakpoint_at))
@@ -4164,7 +4161,7 @@ bpstat_stop_status (struct address_space *aspace,
   else if (removed_any)
     update_global_location_list (0);
 
-  return root_bs->next;
+  return bs_head;
 }
 
 static void

Re: [patch] Code cleanup: bpstat_alloc calling semantics standardization

[Tom Tromey]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Tom> Just to be clear, I assume this is obsoleted by Pedro's other fix for PR
Tom> 11371.

Jan> It is no longer a prerequisite for the PR 11371 fix.

Jan> Still it is a valid code cleanup.  Which is not much of worth.

Once you have written a cleanup, it might as well go in.

Jan> Updated the patch.

I think it looks good, but I would prefer that Pedro say yes or no.

Tom

Re: [patch] Code cleanup: bpstat_alloc calling semantics standardization

[Pedro Alves]

On Tuesday 24 August 2010 22:33:31, Tom Tromey wrote:

> I think it looks good, but I would prefer that Pedro say yes or no.

It looks fine to me, thanks.

-- 
Pedro Alves

Re: [patch] Code cleanup: bpstat_alloc calling semantics standardization

[Jan Kratochvil]

On Wed, 25 Aug 2010 00:09:05 +0200, Pedro Alves wrote:
> On Tuesday 24 August 2010 22:33:31, Tom Tromey wrote:
> 
> > I think it looks good, but I would prefer that Pedro say yes or no.
> 
> It looks fine to me, thanks.

Checked-in:
	http://sourceware.org/ml/gdb-cvs/2010-08/msg00189.html


Thanks,
Jan