On Sat, Feb 18, 2012 at 01:23:36PM -0800, Philip Guenther wrote:
> On Sat, Feb 18, 2012 at 12:20 PM, Arto Jonsson <ajons...@kapsi.fi> wrote:
> > Use queue(3) macros instead of own internal queue implementation. No
> > functional change.
> ...
> > -typedef struct queue {
> > - B B B struct queue B *next;
> > - B B B str_t B B B B B data;
> > -} queue_t;
>
> So, the current queue puts the link in the same allocated block as the
> str_t data.
>
> > -static queue_t *q_head, *q_tail;
> > -static int B B B count;
> > +struct q_entry {
> > + B B B str_t* data;
> > + B B B SIMPLEQ_ENTRY(q_entry) q_entries;
> > +};
> > +SIMPLEQ_HEAD(, q_entry) q_head = SIMPLEQ_HEAD_INITIALIZER(q_head);
>
> Why do you switch to putting the links in separate blocks from the
> str_t data? I.e., why the "*" in "str_t* data" and thus the extra
> mallocs and frees in the code to create and destroy them?
>
>
> Philip Guenther
>
Updated patch.
Index: grep.h
===================================================================
RCS file: /cvs/src/usr.bin/grep/grep.h,v
retrieving revision 1.17
diff -u -r1.17 grep.h
--- grep.h 8 Jul 2011 01:20:24 -0000 1.17
+++ grep.h 19 Feb 2012 07:39:30 -0000
@@ -89,7 +89,6 @@
void fgrepcomp(fastgrep_t *, const char *);
/* queue.c */
-void initqueue(void);
void enqueue(str_t *x);
void printqueue(void);
void clearqueue(void);
Index: queue.c
===================================================================
RCS file: /cvs/src/usr.bin/grep/queue.c,v
retrieving revision 1.6
diff -u -r1.6 queue.c
--- queue.c 8 Jul 2011 01:20:24 -0000 1.6
+++ queue.c 19 Feb 2012 07:39:30 -0000
@@ -26,97 +26,66 @@
* SUCH DAMAGE.
*/
-/*
- * A really poor man's queue. It does only what it has to and gets out of
- * Dodge.
- */
-
#include <sys/param.h>
+#include <sys/queue.h>
#include <stdlib.h>
#include <string.h>
#include "grep.h"
-typedef struct queue {
- struct queue *next;
- str_t data;
-} queue_t;
-
-static queue_t *q_head, *q_tail;
-static int count;
+struct q_entry {
+ SIMPLEQ_ENTRY(q_entry) q_entries;
+ str_t data;
+};
+SIMPLEQ_HEAD(, q_entry) q_head = SIMPLEQ_HEAD_INITIALIZER(q_head);
-static queue_t *dequeue(void);
-
-void
-initqueue(void)
-{
- q_head = q_tail = NULL;
-}
-
-static void
-free_item(queue_t *item)
-{
- free(item);
-}
+static int count;
+static struct q_entry *dequeue(void);
void
enqueue(str_t *x)
{
- queue_t *item;
+ struct q_entry* e;
- item = grep_malloc(sizeof *item + x->len);
- item->data.len = x->len;
- item->data.line_no = x->line_no;
- item->data.off = x->off;
- item->data.dat = (char *)item + sizeof *item;
- memcpy(item->data.dat, x->dat, x->len);
- item->data.file = x->file;
- item->next = NULL;
-
- if (!q_head) {
- q_head = q_tail = item;
- } else {
- q_tail->next = item;
- q_tail = item;
- }
+ e = grep_malloc(sizeof(struct q_entry) + x->len);
+ e->data.len = x->len;
+ e->data.line_no = x->line_no;
+ e->data.off = x->off;
+ e->data.dat = (char *)e + sizeof(struct q_entry);
+ memcpy(e->data.dat, x->dat, x->len);
+ e->data.file = x->file;
+
+ SIMPLEQ_INSERT_TAIL(&q_head, e, q_entries);
if (++count > Bflag)
- free_item(dequeue());
+ free(dequeue());
}
-static queue_t *
+struct q_entry *
dequeue(void)
{
- queue_t *item;
-
- if (q_head == NULL)
- return NULL;
-
+ struct q_entry *e;
--count;
- item = q_head;
- q_head = item->next;
- if (q_head == NULL)
- q_tail = NULL;
- return item;
+ e = SIMPLEQ_FIRST(&q_head);
+ SIMPLEQ_REMOVE_HEAD(&q_head, q_entries);
+ return e;
}
void
printqueue(void)
{
- queue_t *item;
-
- while ((item = dequeue()) != NULL) {
- printline(&item->data, '-', NULL);
- free_item(item);
+ struct q_entry *e;
+ while (!SIMPLEQ_EMPTY(&q_head)) {
+ e = dequeue();
+ printline(&e->data, '-', NULL);
+ free(e);
}
}
void
clearqueue(void)
{
- queue_t *item;
-
- while ((item = dequeue()) != NULL)
- free_item(item);
+ while (!SIMPLEQ_EMPTY(&q_head))
+ free(dequeue());
}
Index: util.c
===================================================================
RCS file: /cvs/src/usr.bin/grep/util.c,v
retrieving revision 1.42
diff -u -r1.42 util.c
--- util.c 17 Jul 2011 19:39:21 -0000 1.42
+++ util.c 19 Feb 2012 07:39:30 -0000
@@ -119,8 +119,6 @@
tail = 0;
ln.off = -1;
- if (Bflag > 0)
- initqueue();
for (c = 0; c == 0 || !(lflag || qflag); ) {
ln.off += ln.len + 1;
if ((ln.dat = grep_fgetln(f, &ln.len)) == NULL)
