GNU bug report logs - #25332
[PATCH] Change while loops to for loops.

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 25332 in the body.
You can then email your comments to 25332 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Chris Gregory <czipperz <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Change while loops to for loops.
Date: Mon, 02 Jan 2017 00:26:07 -0800

This patch changes while loops to for loops to make the code more
consistent.
-- 
Chris Gregory

diff --git a/src/ralloc.c b/src/ralloc.c
index 8a3d2b797f..ccc019618c 100644
--- a/src/ralloc.c
+++ b/src/ralloc.c
@@ -222,13 +222,10 @@ obtain (void *address, size_t size)
 
   /* If we can't fit SIZE bytes in that heap,
      try successive later heaps.  */
-  while (heap && (char *) address + size > (char *) heap->end)
-    {
-      heap = heap->next;
-      if (heap == NIL_HEAP)
-	break;
-      address = heap->bloc_start;
-    }
+  for (; heap && (char *) address + size > (char *) heap->end;
+       address = heap->bloc_start)
+    if ((heap = heap->next) == NIL_HEAP)
+      break;
 
   /* If we can't fit them within any existing heap,
      get more space.  */
@@ -350,20 +347,16 @@ relinquish (void)
 static bloc_ptr
 find_bloc (void **ptr)
 {
-  bloc_ptr p = first_bloc;
-
-  while (p != NIL_BLOC)
-    {
-      /* Consistency check. Don't return inconsistent blocs.
-	 Don't abort here, as callers might be expecting this, but
-	 callers that always expect a bloc to be returned should abort
-	 if one isn't to avoid a memory corruption bug that is
-	 difficult to track down.  */
-      if (p->variable == ptr && p->data == *ptr)
-	return p;
-
-      p = p->next;
-    }
+  bloc_ptr p;
+
+  for (p = first_bloc; p != NIL_BLOC; p = p->next)
+    /* Consistency check. Don't return inconsistent blocs.
+       Don't abort here, as callers might be expecting this, but
+       callers that always expect a bloc to be returned should abort
+       if one isn't to avoid a memory corruption bug that is
+       difficult to track down.  */
+    if (p->variable == ptr && p->data == *ptr)
+      break;
 
   return p;
 }
@@ -430,13 +423,13 @@ get_bloc (size_t size)
 static int
 relocate_blocs (bloc_ptr bloc, heap_ptr heap, void *address)
 {
-  bloc_ptr b = bloc;
+  bloc_ptr b;
 
   /* No need to ever call this if arena is frozen, bug somewhere!  */
   if (r_alloc_freeze_level)
     emacs_abort ();
 
-  while (b)
+  for (b = bloc; b; b = b->next)
     {
       /* If bloc B won't fit within HEAP,
 	 move to the next heap and try again.  */
@@ -452,17 +445,13 @@ relocate_blocs (bloc_ptr bloc, heap_ptr heap, void *address)
 	 get enough new space to hold BLOC and all following blocs.  */
       if (heap == NIL_HEAP)
 	{
-	  bloc_ptr tb = b;
+	  bloc_ptr tb;
 	  size_t s = 0;
 
 	  /* Add up the size of all the following blocs.  */
-	  while (tb != NIL_BLOC)
-	    {
-	      if (tb->variable)
-		s += tb->size;
-
-	      tb = tb->next;
-	    }
+	  for (tb = b; tb != NIL_BLOC; tb = tb->next)
+	    if (tb->variable)
+	      s += tb->size;
 
 	  /* Get that space.  */
 	  address = obtain (address, s);
@@ -477,7 +466,6 @@ relocate_blocs (bloc_ptr bloc, heap_ptr heap, void *address)
       b->new_data = address;
       if (b->variable)
 	address = (char *) address + b->size;
-      b = b->next;
     }
 
   return 1;
@@ -535,13 +523,11 @@ update_heap_bloc_correspondence (bloc_ptr bloc, heap_ptr heap)
 
   /* If there are any remaining heaps and no blocs left,
      mark those heaps as empty.  */
-  heap = heap->next;
-  while (heap)
+  for (heap = heap->next; heap; heap = heap->next)
     {
       heap->first_bloc = NIL_BLOC;
       heap->last_bloc = NIL_BLOC;
       heap->free = heap->bloc_start;
-      heap = heap->next;
     }
 }
 
@@ -578,12 +564,9 @@ resize_bloc (bloc_ptr bloc, size_t size)
   /* Note that bloc could be moved into the previous heap.  */
   address = (bloc->prev ? (char *) bloc->prev->data + bloc->prev->size
 	     : (char *) first_heap->bloc_start);
-  while (heap)
-    {
-      if (heap->bloc_start <= address && address <= heap->end)
-	break;
-      heap = heap->prev;
-    }
+  for (; heap; heap = heap->next)
+    if (heap->bloc_start <= address && address <= heap->end)
+      break;
 
   if (! relocate_blocs (bloc, heap, address))
     {
@@ -1084,7 +1067,7 @@ r_alloc_check (void)
       if (pb && pb->data + pb->size != b->data)
 	{
 	  assert (ph && b->data == h->bloc_start);
-	  while (ph)
+	  for (; ph; ph = ph->prev)
 	    {
 	      if (ph->bloc_start <= pb->data
 		  && pb->data + pb->size <= ph->end)
@@ -1093,10 +1076,7 @@ r_alloc_check (void)
 		  break;
 		}
 	      else
-		{
-		  assert (ph->bloc_start + b->size > ph->end);
-		}
-	      ph = ph->prev;
+		assert (ph->bloc_start + b->size > ph->end);
 	    }
 	}
       pb = b;
@@ -1120,18 +1100,14 @@ r_alloc_check (void)
 void
 r_alloc_reset_variable (void **old, void **new)
 {
-  bloc_ptr bloc = first_bloc;
+  bloc_ptr bloc;
 
   /* Find the bloc that corresponds to the data pointed to by pointer.
      find_bloc cannot be used, as it has internal consistency checks
      which fail when the variable needs resetting.  */
-  while (bloc != NIL_BLOC)
-    {
-      if (bloc->data == *new)
-	break;
-
-      bloc = bloc->next;
-    }
+  for (bloc = first_bloc; bloc != NIL_BLOC; bloc = bloc->next)
+    if (bloc->data == *new)
+      break;
 
   if (bloc == NIL_BLOC || bloc->variable != old)
     emacs_abort (); /* Already freed? OLD not originally used to allocate?  */

Message #8 received at 25332 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Chris Gregory <czipperz <at> gmail.com>
Cc: 25332 <at> debbugs.gnu.org
Subject: Re: bug#25332: [PATCH] Change while loops to for loops.
Date: Sat, 07 Jan 2017 10:30:12 +0200

tags 25332 notabug
close 25332
thanks

> From: Chris Gregory <czipperz <at> gmail.com>
> Date: Mon, 02 Jan 2017 00:26:07 -0800
> 
> This patch changes while loops to for loops to make the code more
> consistent.

I don't think I understand the rationale.  Why is using 'while'
inconsistent?

In any case, please don't bother making style changes in ralloc.c, as
that file should almost never be used in Emacs, except in some
marginal configurations, and we actually would like to get rid of it
altogether.  It's therefore a waste of effort to try to make its style
better.

Message #15 received at 25332 <at> debbugs.gnu.org (full text, mbox):

From: Chris Gregory <czipperz <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 25332 <at> debbugs.gnu.org
Subject: Re: bug#25332: [PATCH] Change while loops to for loops.
Date: Sun, 8 Jan 2017 13:24:04 -0800

[Message part 1 (text/plain, inline)]

There are places in ralloc.c where a for loop is used to express a loop,
and places where a while loop is used with the increment operation at the
end of it.  This would just simplify all code so that it was consistently
using a for loop for this purpose.

Without this patch here is a good usage of for:

  heap_ptr heap;

  for (heap = last_heap; heap; heap = heap->prev)
    {
      if (heap->start <= address && address <= heap->end)
return heap;
    }

Here is a usage of while that should be a for loop like the previous
example:

  bloc_ptr p = first_bloc;

  while (p != NIL_BLOC)
    {
      /* Consistency check. Don't return inconsistent blocs.
Don't abort here, as callers might be expecting this, but
callers that always expect a bloc to be returned should abort
if one isn't to avoid a memory corruption bug that is
difficult to track down.  */
      if (p->variable == ptr && p->data == *ptr)
return p;

      p = p->next;
    }

Recommended new code:

  bloc_ptr p;

  for (p = first_bloc; p != NIL_BLOC; p = p->next)
    {
      /* Consistency check. Don't return inconsistent blocs.
Don't abort here, as callers might be expecting this, but
callers that always expect a bloc to be returned should abort
if one isn't to avoid a memory corruption bug that is
difficult to track down.  */
      if (p->variable == ptr && p->data == *ptr)
return p;
    }

This style exact loop is even used later on (line 509 in
update_heap_bloc_correspondence()):

  /* Advance through blocs one by one.  */
  for (b = bloc; b != NIL_BLOC; b = b->next)

-- 
Chris Gregory

On Sat, Jan 7, 2017 at 12:30 AM, Eli Zaretskii <eliz <at> gnu.org> wrote:

> tags 25332 notabug
> close 25332
> thanks
>
> > From: Chris Gregory <czipperz <at> gmail.com>
> > Date: Mon, 02 Jan 2017 00:26:07 -0800
> >
> > This patch changes while loops to for loops to make the code more
> > consistent.
>
> I don't think I understand the rationale.  Why is using 'while'
> inconsistent?
>
> In any case, please don't bother making style changes in ralloc.c, as
> that file should almost never be used in Emacs, except in some
> marginal configurations, and we actually would like to get rid of it
> altogether.  It's therefore a waste of effort to try to make its style
> better.
>

[Message part 2 (text/html, inline)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.