patchPSPP - Patches: patch #5709, Augmented Balanced Tree data...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

patch #5709: Augmented Balanced Tree data structure

Submitter:  Ben Pfaff <blp>
Submitted:  Thu 25 Jan 2007 05:15:08 AM UTC
   
 
Category:  None Item Group:  None
Status:  Done Assigned to:  blp
Open/Closed:  Closed

Fri 26 Jan 2007 03:34:34 AM UTC, comment #5: 

I checked this in, with the suggested changes.

Ben Pfaff <blp>
Group administrator
Thu 25 Jan 2007 11:00:37 PM UTC, comment #4: 


>I would have probably written something like: [...]


I agree that your version is clearer.  I'll use it instead of mine.

Ben Pfaff <blp>
Group administrator
Thu 25 Jan 2007 10:21:20 PM UTC, comment #3: 

You're right.

I would have probably written something like:

static void
reverse (int *values, size_t cnt)
{
 size_t i = 0;
 size_t j = cnt - 1;

 while (j > i )
   swap (values[i++], values[j--]);
}

John Darrington <jmd>
Group administrator
Thu 25 Jan 2007 04:52:40 PM UTC, comment #2: 

Thanks.  I'll fix these problems before I check in.  Except:

>If I'm reading this correctly, then the comment doesn't agree with
>the code. It doesn't reverse the order of CNT integers, but
>reverses the order of each pair of integers.


Take another look.  It swaps the first and last integer, then the second and second-to-last, and so on.  The overall effect is to reverse the sequence.

Is there another way to write it that you'd find clearer?

Ben Pfaff <blp>
Group administrator
Thu 25 Jan 2007 07:11:16 AM UTC, comment #1: 

Hey!  This could be useful in some of my current research!
I haven't tried it, but looking through the patch ...

abt_init: might the aux parameter be better declared const?

+struct abt_node *
+abt_first (const struct abt *abt)
+{
+  struct abt_node *p = abt->root;
+  if (p != NULL)
+    while (p->down[0] != NULL)
+      p = p->down[0];
+  return (struct abt_node *) p;
+}

Is this cast neccesary ?



abt-test.c: This mangled line appears:
 This program is distributed in the hope that it wiabt be useful
                                                  ^^^^



static unsigned
factorial (unsigned n)
{
  unsigned value = 1;

I don't like qualifiers without a type.  I thought gcc would have given a warning about this.




+/* Reverses the order of the CNT integers starting at VALUES. */
+static void
+reverse (int *values, size_t cnt)
+{
+  for (; cnt > 1; cnt -= 2, values++)
+    swap (values, &values[cnt - 1]);
+}

If I'm reading this correctly, then the comment doesn't agree with the code. It doesn't reverse the order of CNT integers, but reverses the order of each pair of integers.

John Darrington <jmd>
Group administrator
Thu 25 Jan 2007 05:15:08 AM UTC, original submission:  

Here's a data structure I'm planning to use as part of the flexifile implementation.  Comments?

Ben Pfaff <blp>
Group administrator

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attached Files
file #11860:  abt.patch added by blp (42KiB - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by jmd (Posted a comment)
  • -email is unavailable- added by blp (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2007-01-26 blp StatusWorks For Me Done
        Open/ClosedOpen Closed
    2007-01-25 jmd StatusReady For Test/Review Works For Me
        Assigned tojmd blp
    2007-01-25 blp Attached File- Added abt.patch, #11860

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code