[PATCH] libpipe/pq.c

[Ognyan Kulev]

Hi,

In `libpipe/pq.c:packet_set_ports' when the new ports array is bigger 
than `packet->ports' a new ports array is malloced but it doesn't 
replace the old one.  The first patch fix this.

In some places in the file `sizeof (mach_port_t *)' is used while 
obviously `sizeof (mach_port_t)' is meant.  The second patch fix this.

The third patch deals with reusing the packets in function `pq_queue'. 
Let's suppose we do `pq_dequeue' for a packet with ports: 
`packet_dealloc_ports' is called but `packet->num_ports' is not changed 
and the packet go in the list of the free packets.  If `pq_queue' reuse 
this packet `packet->num_ports' tells that some ports are in the packet 
and a subsequent call to `packet_set_ports' will deallocate these ports 
again.  So when reusing packets we must make sure that `num_ports' is 
set to 0.

I don't know how this library is used so it is possible that a part of 
its interfaces demand that ports are already removed by 
`packet_read_ports'.  But why then `pq_dequeue' calls 
`packet_dealloc_ports'?  So i think the correct is to set num_ports to zero.

Instead of talking more i'll just mention that the patch initializes 
`buf_start' and `buf_end' too.

And now some general questions:

There are many places where `bcopy' is used instead of `memcpy'.  The 
reason for preferring `memcpy' is that `memcpy' doesn't check for 
overlapping memory segments, while `bcopy' and `memmove' do.  Are you 
accepting patches for such issues?

The second question is about using mmap/munmap instead of 
vm_allocate/vm_deallocate.  In pq.[ch] they are mixed, there is no 
consistency.  What is the reason to prefer `mmap' to `vm_allocate'?  Is 
there a policy about that?

Regards
--
Ognyan Kulev <ogi@fmi.uni-sofia.bg>, "\"Programmer\""

--- pq.c.orig   Wed Apr 10 12:45:10 2002
+++ pq.c        Wed Apr 10 12:49:44 2002
@@ -299,6 +299,7 @@ packet_set_ports (struct packet *packet,
       if (! new_ports)
        return ENOMEM;
       free (packet->ports);
+      packet->ports = new_ports;
       packet->ports_alloced = num_ports;
     }
   bcopy (ports, packet->ports, sizeof (mach_port_t *) * num_ports);

--- pq.c.ports  Wed Apr 10 12:49:44 2002
+++ pq.c        Wed Apr 10 12:50:21 2002
@@ -295,14 +295,14 @@ packet_set_ports (struct packet *packet,
     packet_dealloc_ports (packet);
   if (num_ports > packet->ports_alloced)
     {
-      mach_port_t *new_ports = malloc (sizeof (mach_port_t *) * num_ports);
+      mach_port_t *new_ports = malloc (sizeof (mach_port_t) * num_ports);
       if (! new_ports)
        return ENOMEM;
       free (packet->ports);
       packet->ports = new_ports;
       packet->ports_alloced = num_ports;
     }
-  bcopy (ports, packet->ports, sizeof (mach_port_t *) * num_ports);
+  bcopy (ports, packet->ports, sizeof (mach_port_t) * num_ports);
   packet->num_ports = num_ports;
   return 0;
 }
@@ -313,7 +313,7 @@ error_t
 packet_read_ports (struct packet *packet,
                   mach_port_t **ports, size_t *num_ports)
 {
-  int length = packet->num_ports * sizeof (mach_port_t *);
+  int length = packet->num_ports * sizeof (mach_port_t);
   if (*num_ports < packet->num_ports)
     {
       *ports = mmap (0, length, PROT_READ|PROT_WRITE, MAP_ANON, 0, 0);

--- pq.c.mach   Wed Apr 10 13:02:11 2002
+++ pq.c        Wed Apr 10 13:02:15 2002
@@ -132,7 +132,11 @@ pq_queue (struct pq *pq, unsigned type, 
       packet->buf_vm_alloced = 0;
     }
   else
-    pq->free = packet->next;
+    {
+      pq->free = packet->next;
+      packet->num_ports = 0;
+      packet->buf_start = packet->buf_end = packet->buf;
+    }
 
   packet->type = type;
   packet->source = source;