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 <[EMAIL PROTECTED]>, "\"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;
