[Simh] Problem in eth_host_devices() ?

[Galen Tackett]

In the past I've posted to the mailing list a few
times concerning not being able to use en1 on a simh
Vax running under Mac OS X (V10.3.9). I've done some
investigating and may have turned up something.

(Things get pretty detailed from this point. Feel free
to skim over the details if you like.)

I wonder if I've found a problem with the routine
eth_host_devices() in sim_ether.c. It's a short
routine so I'll include the code here. It's been
slightly reformatted, but if you spot any other
differences I probably made a cut/paste error somehow.

---Start of excerpt from sim_ether.c---
int eth_host_devices(int used, int max, ETH_LIST*
list)
{
  pcap_t* conn;
  int i, j, datalink;
  char errbuf[PCAP_ERRBUF_SIZE];

  for (i=0; i<used; ++i) {
    /* Cull any non-ethernet interface types */
    conn = pcap_open_live(list[i].name, 
                          ETH_MAX_PACKET,
                          ETH_PROMISC,
                          PCAP_READ_TIMEOUT,
                          errbuf);
    if (NULL != conn) 
      datalink = pcap_datalink(conn),
pcap_close(conn);
    if ((NULL == conn) || (datalink != DLT_EN10MB)) {
      for (j=i+1; j<used; ++j)
        list[i+j] = list[i+j+1];
      --used;
    }
  }
  return used;
}

---End of excerpt---

I believe what's intended here is to remove non-usable
network interfaces from the list of interfaces at
*list AKA list[]. (Here, a device is unusable if
pcap_open_live can't open the interface, or if the
interface's datalink type is not Ethernet.) The inner
for() loop is supposed to do the removal:

      for (j=i+1; j<used; ++j)
        list[i+j] = list[i+j+1];

But it appears to me that this loop SHOULD read:

      for (j=i; j<used; ++j)
        list[j] = list[j+1];

Consider the following list of devices (the names were
arbitrarily chosen though some you might see on a real
system):

list[0]:  en0
list[1]:  foo0
list[2]:  fw0
list[3]:  en1
list[4]:  baz0
list[5]:  baz1
.
.
.

With the code as written, suppose we're at i=1 and we
call pcap_open_live() against foo0. pcap_open_live()
succeeds, so conn is now non-NULL. However, we look at
the datalink type for foo0 and find that it is not
equal to DLT_EN10MB. This means foo0 is not an
Ethernet-type device and we don't want to use it. So
we'll go into the inner loop to remove it from the
list.

As we start the inner loop, j will be set to i+1, that
is, 2. This statement comes next:
     list[i+j] = list[i+j+1]

Now, i+j is 3 and i+j+1 is 4. This means that list[3]
(en1) gets overwritten with baz0, then list[4] gets
overwritten with list[5] (baz1), etc. When we finish
the loop we'll have:

list[0]:  en0
list[1]:  foo0
list[2]:  fw0
list[3]:  baz0
list[4]:  baz1
.
.
.

So instead of dropping foo0 from the list, we have
mistakenly dropped en1.

As the code is written, the simh command SHO XQ ETH
omits my en1 Ethernet interface but includes, say, an
unusable foo0. With my change, en1 shows up and foo0
disappears.

Can some of you check for similar behavior on OS X or
other host systems?