Patchwork [PATCHv3,19/20] eepro100: Remove C++ comments

login
register
mail settings
Submitter Stefan Weil
Date March 2, 2010, 9:37 p.m.
Message ID <1267565880-18382-19-git-send-email-weil@mail.berlios.de>
Download mbox | patch
Permalink /patch/46729/
State New
Headers show

Comments

Stefan Weil - March 2, 2010, 9:37 p.m.
C++ comments are unwanted, so this is fixed here.

* Replace C++ comments by C comments.
* Put code which was deactivated by a C++ comment in #if 0...#endif.

Signed-off-by: Stefan Weil <weil@mail.berlios.de>
---
 hw/eepro100.c |  185 +++++++++++++++++++++++++++++++++++++++------------------
 1 files changed, 126 insertions(+), 59 deletions(-)
Michael S. Tsirkin - March 3, 2010, 11:47 a.m.
On Tue, Mar 02, 2010 at 10:37:59PM +0100, Stefan Weil wrote:
> C++ comments are unwanted, so this is fixed here.
> 
> * Replace C++ comments by C comments.
> * Put code which was deactivated by a C++ comment in #if 0...#endif.
> 
> Signed-off-by: Stefan Weil <weil@mail.berlios.de>


It would be nice to add some documentation to commented
blocks, explaining why we still keep them around
and what happens if they are uncommented.

> ---
>  hw/eepro100.c |  185 +++++++++++++++++++++++++++++++++++++++------------------
>  1 files changed, 126 insertions(+), 59 deletions(-)
> 
> diff --git a/hw/eepro100.c b/hw/eepro100.c
> index 0f07b70..43a9060 100644
> --- a/hw/eepro100.c
> +++ b/hw/eepro100.c
> @@ -154,11 +154,13 @@ typedef struct {
>      uint16_t tcb_bytes;         /* transmit command block byte count (in lower 14 bits */
>      uint8_t tx_threshold;       /* transmit threshold */
>      uint8_t tbd_count;          /* TBD number */
> -    //~ /* This constitutes two "TBD" entries: hdr and data */
> -    //~ uint32_t tx_buf_addr0;  /* void *, header of frame to be transmitted.  */
> -    //~ int32_t  tx_buf_size0;  /* Length of Tx hdr. */
> -    //~ uint32_t tx_buf_addr1;  /* void *, data to be transmitted.  */
> -    //~ int32_t  tx_buf_size1;  /* Length of Tx data. */
> +#if 0
> +    /* This constitutes two "TBD" entries: hdr and data */
> +    uint32_t tx_buf_addr0;  /* void *, header of frame to be transmitted.  */
> +    int32_t  tx_buf_size0;  /* Length of Tx hdr. */
> +    uint32_t tx_buf_addr1;  /* void *, data to be transmitted.  */
> +    int32_t  tx_buf_size1;  /* Length of Tx data. */
> +#endif

Is it likely we'll uncomment this at some point?
I note that you now use sizeof this structure: will
adding these fields break anything?

>  } eepro100_tx_t;
>  
>  /* Receive frame descriptor. */
> @@ -398,7 +400,9 @@ static void eepro100_interrupt(EEPRO100State * s, uint8_t status)
>      s->mem[SCBAck] |= status;
>      status = s->scb_stat = s->mem[SCBAck];
>      status &= (mask | 0x0f);
> -    //~ status &= (~s->mem[SCBIntmask] | 0x0xf);
> +#if 0
> +    status &= (~s->mem[SCBIntmask] | 0x0xf);
> +#endif
>      if (status && (mask & 0x01)) {
>          /* SCB mask and SCB Bit M do not disable interrupt. */
>          enable_interrupt(s);
> @@ -477,9 +481,11 @@ static void pci_reset(EEPRO100State * s)
>      pci_config_set_class(pci_conf, PCI_CLASS_NETWORK_ETHERNET);
>      /* PCI Cache Line Size */
>      /* check cache line size!!! */
> -    //~ PCI_CONFIG_8(0x0c, 0x00);
> +#if 0
> +    PCI_CONFIG_8(0x0c, 0x00);
> +#endif

It's safe to kill the above, 0 is the default anyway.

>      /* PCI Latency Timer */
> -    PCI_CONFIG_8(PCI_LATENCY_TIMER, 0x20);   // latency timer = 32 clocks
> +    PCI_CONFIG_8(PCI_LATENCY_TIMER, 0x20);   /* latency timer = 32 clocks */
>      /* PCI Header Type */
>      /* BIST (built-in self test) */
>      /* Expansion ROM Base Address (depends on boot disable!!!) */
> @@ -492,7 +498,7 @@ static void pci_reset(EEPRO100State * s)
>      /* Interrupt Line */
>      /* Interrupt Pin */
>      /* TODO: RST# value should be 0 */
> -    PCI_CONFIG_8(PCI_INTERRUPT_PIN, 1);      // interrupt pin 0
> +    PCI_CONFIG_8(PCI_INTERRUPT_PIN, 1);      /* interrupt pin 0 */
>      /* Minimum Grant */
>      PCI_CONFIG_8(PCI_MIN_GNT, 0x08);
>      /* Maximum Latency */
> @@ -500,20 +506,20 @@ static void pci_reset(EEPRO100State * s)
>  
>      switch (device) {
>      case i82550:
> -        // TODO: check device id.
> +        /* TODO: check device id. */
>          pci_config_set_device_id(pci_conf, PCI_DEVICE_ID_INTEL_82551IT);
>          /* Revision ID: 0x0c, 0x0d, 0x0e. */
>          PCI_CONFIG_8(PCI_REVISION_ID, 0x0e);
> -        // TODO: check size of statistical counters.
> +        /* TODO: check size of statistical counters. */
>          s->stats_size = 80;
> -        // TODO: check extended tcb support.
> +        /* TODO: check extended tcb support. */
>          s->has_extended_tcb_support = 1;
>          break;
>      case i82551:
>          pci_config_set_device_id(pci_conf, PCI_DEVICE_ID_INTEL_82551IT);
>          /* Revision ID: 0x0f, 0x10. */
>          PCI_CONFIG_8(PCI_REVISION_ID, 0x0f);
> -        // TODO: check size of statistical counters.
> +        /* TODO: check size of statistical counters. */
>          s->stats_size = 80;
>          s->has_extended_tcb_support = 1;
>          break;
> @@ -572,7 +578,7 @@ static void pci_reset(EEPRO100State * s)
>          PCI_CONFIG_16(PCI_STATUS, PCI_STATUS_DEVSEL_MEDIUM |
>                                    PCI_STATUS_FAST_BACK | PCI_STATUS_CAP_LIST);
>          PCI_CONFIG_8(PCI_REVISION_ID, 0x08);
> -        // TODO: Windows wants revision id 0x0c.
> +        /* TODO: Windows wants revision id 0x0c. */
>          PCI_CONFIG_8(PCI_REVISION_ID, 0x0c);
>  #if EEPROM_SIZE > 0
>          PCI_CONFIG_16(PCI_SUBSYSTEM_VENDOR_ID, 0x8086);
> @@ -590,7 +596,7 @@ static void pci_reset(EEPRO100State * s)
>          s->has_extended_tcb_support = 1;
>          break;
>      case i82562:
> -        // TODO: check device id.
> +        /* TODO: check device id. */
>          pci_config_set_device_id(pci_conf, PCI_DEVICE_ID_INTEL_82551IT);
>          /* TODO: wrong revision id. */
>          PCI_CONFIG_8(PCI_REVISION_ID, 0x0e);
> @@ -637,14 +643,16 @@ static void pci_reset(EEPRO100State * s)
>  
>  #if EEPROM_SIZE > 0
>      if (device == i82557C || device == i82558B || device == i82559C) {
> -        // TODO: get vendor id from EEPROM for i82557C or later.
> -        // TODO: get device id from EEPROM for i82557C or later.
> -        // TODO: status bit 4 can be disabled by EEPROM for i82558, i82559.
> -        // TODO: header type is determined by EEPROM for i82559.
> -        // TODO: get subsystem id from EEPROM for i82557C or later.
> -        // TODO: get subsystem vendor id from EEPROM for i82557C or later.
> -        // TODO: exp. rom baddr depends on a bit in EEPROM for i82558 or later.
> -        // TODO: capability pointer depends on EEPROM for i82558.
> +        /*
> +        TODO: get vendor id from EEPROM for i82557C or later.
> +        TODO: get device id from EEPROM for i82557C or later.
> +        TODO: status bit 4 can be disabled by EEPROM for i82558, i82559.
> +        TODO: header type is determined by EEPROM for i82559.
> +        TODO: get subsystem id from EEPROM for i82557C or later.
> +        TODO: get subsystem vendor id from EEPROM for i82557C or later.
> +        TODO: exp. rom baddr depends on a bit in EEPROM for i82558 or later.
> +        TODO: capability pointer depends on EEPROM for i82558.
> +        */
>          logout("Get device id and revision from EEPROM!!!\n");
>      }
>  #endif /* EEPROM_SIZE > 0 */
> @@ -654,7 +662,9 @@ static void nic_selective_reset(EEPRO100State * s)
>  {
>      size_t i;
>      uint16_t *eeprom_contents = eeprom93xx_data(s->eeprom);
> -    //~ eeprom93xx_reset(s->eeprom);
> +#if 0
> +    eeprom93xx_reset(s->eeprom);
> +#endif
>      memcpy(eeprom_contents, s->conf.macaddr.a, 6);
>      eeprom_contents[EEPROM_ID] = EEPROM_ID_VALID;
>      if (s->device == i82557B || s->device == i82557C)
> @@ -738,7 +748,7 @@ static void eepro100_write_status(EEPRO100State * s, uint16_t val)
>  static uint16_t eepro100_read_command(EEPRO100State * s)
>  {
>      uint16_t val = 0xffff;
> -    //~ TRACE(OTHER, logout("val=0x%04x\n", val));
> +    TRACE(OTHER, logout("val=0x%04x\n", val));
>      return val;
>  }
>  #endif
> @@ -793,9 +803,11 @@ static void dump_statistics(EEPRO100State * s)
>      stl_le_phys(s->statsaddr + 36, s->statistics.rx_good_frames);
>      stl_le_phys(s->statsaddr + 48, s->statistics.rx_resource_errors);
>      stl_le_phys(s->statsaddr + 60, s->statistics.rx_short_frame_errors);
> -    //~ stw_le_phys(s->statsaddr + 76, s->statistics.xmt_tco_frames);
> -    //~ stw_le_phys(s->statsaddr + 78, s->statistics.rcv_tco_frames);
> -    //~ missing("CU dump statistical counters");
> +#if 0
> +    stw_le_phys(s->statsaddr + 76, s->statistics.xmt_tco_frames);
> +    stw_le_phys(s->statsaddr + 78, s->statistics.rcv_tco_frames);
> +    missing("CU dump statistical counters");
> +#endif
>  }
>  
>  static void read_cb(EEPRO100State *s)
> @@ -832,7 +844,9 @@ static void tx_command(EEPRO100State *s)
>      while (size < tcb_bytes) {
>          uint32_t tx_buffer_address = ldl_phys(tbd_address);
>          uint16_t tx_buffer_size = lduw_phys(tbd_address + 4);
> -        //~ uint16_t tx_buffer_el = lduw_phys(tbd_address + 6);
> +#if 0
> +        uint16_t tx_buffer_el = lduw_phys(tbd_address + 6);
> +#endif
>          tbd_address += 8;
>          TRACE(RXTX, logout
>              ("TBD (simplified mode): buffer address 0x%08x, size 0x%04x\n",
> @@ -889,7 +903,9 @@ static void tx_command(EEPRO100State *s)
>      s->statistics.tx_good_frames++;
>      /* Transmit with bad status would raise an CX/TNO interrupt.
>       * (82557 only). Emulation never has bad status. */
> -    //~ eepro100_cx_interrupt(s);
> +#if 0
> +    eepro100_cx_interrupt(s);
> +#endif
>  }
>  
>  static void set_multicast_list(EEPRO100State *s)
> @@ -1016,7 +1032,9 @@ static void eepro100_cu_command(EEPRO100State * s, uint8_t val)
>              logout("bad CU resume from CU state %u\n", get_cu_state(s));
>              /* Workaround for bad Linux eepro100 driver which resumes
>               * from idle state. */
> -            //~ missing("cu resume");
> +#if 0
> +            missing("cu resume");
> +#endif
>              set_cu_state(s, cu_suspended);
>          }
>          if (get_cu_state(s) == cu_suspended) {
> @@ -1067,7 +1085,9 @@ static void eepro100_ru_command(EEPRO100State * s, uint8_t val)
>          /* RU start. */
>          if (get_ru_state(s) != ru_idle) {
>              logout("RU state is %u, should be %u\n", get_ru_state(s), ru_idle);
> -            //~ assert(!"wrong RU state");
> +#if 0
> +            assert(!"wrong RU state");
> +#endif
>          }
>          set_ru_state(s, ru_ready);
>          s->ru_offset = s->pointer;
> @@ -1078,7 +1098,9 @@ static void eepro100_ru_command(EEPRO100State * s, uint8_t val)
>          if (get_ru_state(s) != ru_suspended) {
>              logout("RU state is %u, should be %u\n", get_ru_state(s),
>                     ru_suspended);
> -            //~ assert(!"wrong RU state");
> +#if 0
> +            assert(!"wrong RU state");
> +#endif
>          }
>          set_ru_state(s, ru_ready);
>          break;
> @@ -1140,7 +1162,9 @@ static void eepro100_write_eeprom(eeprom_t * eeprom, uint8_t val)
>      TRACE(EEPROM, logout("val=0x%02x\n", val));
>  
>      /* mask unwriteable bits */
> -    //~ val = SET_MASKED(val, 0x31, eeprom->value);
> +#if 0
> +    val = SET_MASKED(val, 0x31, eeprom->value);
> +#endif
>  
>      int eecs = ((val & EEPROM_CS) != 0);
>      int eesk = ((val & EEPROM_SK) != 0);
> @@ -1222,7 +1246,9 @@ static void eepro100_write_mdi(EEPRO100State * s, uint32_t val)
>            val, raiseint, mdi_op_name[opcode], phy, reg2name(reg), data));
>      if (phy != 1) {
>          /* Unsupported PHY address. */
> -        //~ logout("phy must be 1 but is %u\n", phy);
> +#if 0
> +        logout("phy must be 1 but is %u\n", phy);
> +#endif
>          data = 0;
>      } else if (opcode != 1 && opcode != 2) {
>          /* Unsupported opcode. */
> @@ -1368,16 +1394,22 @@ static uint8_t eepro100_read1(EEPRO100State * s, uint32_t addr)
>  
>      switch (addr) {
>      case SCBStatus:
> -        //~ val = eepro100_read_status(s);
> +#if 0
> +        val = eepro100_read_status(s);
> +#endif
>          TRACE(OTHER, logout("addr=%s val=0x%02x\n", regname(addr), val));
>          break;
>      case SCBAck:
> -        //~ val = eepro100_read_status(s);
> +#if 0
> +        val = eepro100_read_status(s);
> +#endif
>          TRACE(OTHER, logout("addr=%s val=0x%02x\n", regname(addr), val));
>          break;
>      case SCBCmd:
>          TRACE(OTHER, logout("addr=%s val=0x%02x\n", regname(addr), val));
> -        //~ val = eepro100_read_command(s);
> +#if 0
> +        val = eepro100_read_command(s);
> +#endif
>          break;
>      case SCBIntmask:
>          TRACE(OTHER, logout("addr=%s val=0x%02x\n", regname(addr), val));
> @@ -1413,7 +1445,9 @@ static uint16_t eepro100_read2(EEPRO100State * s, uint32_t addr)
>  
>      switch (addr) {
>      case SCBStatus:
> -        //~ val = eepro100_read_status(s);
> +#if 0
> +        val = eepro100_read_status(s);
> +#endif
>      case SCBCmd:
>          TRACE(OTHER, logout("addr=%s val=0x%04x\n", regname(addr), val));
>          break;
> @@ -1437,11 +1471,15 @@ static uint32_t eepro100_read4(EEPRO100State * s, uint32_t addr)
>  
>      switch (addr) {
>      case SCBStatus:
> -        //~ val = eepro100_read_status(s);
> +#if 0
> +        val = eepro100_read_status(s);
> +#endif
>          TRACE(OTHER, logout("addr=%s val=0x%08x\n", regname(addr), val));
>          break;
>      case SCBPointer:
> -        //~ val = eepro100_read_pointer(s);
> +#if 0
> +        val = eepro100_read_pointer(s);
> +#endif
>          TRACE(OTHER, logout("addr=%s val=0x%08x\n", regname(addr), val));
>          break;
>      case SCBPort:
> @@ -1468,7 +1506,9 @@ static void eepro100_write1(EEPRO100State * s, uint32_t addr, uint8_t val)
>  
>      switch (addr) {
>      case SCBStatus:
> -        //~ eepro100_write_status(s, val);
> +#if 0
> +        eepro100_write_status(s, val);
> +#endif
>          break;
>      case SCBAck:
>          eepro100_acknowledge(s);
> @@ -1508,7 +1548,9 @@ static void eepro100_write2(EEPRO100State * s, uint32_t addr, uint16_t val)
>  
>      switch (addr) {
>      case SCBStatus:
> -        //~ eepro100_write_status(s, val);
> +#if 0
> +        eepro100_write_status(s, val);
> +#endif
>          eepro100_acknowledge(s);
>          break;
>      case SCBCmd:
> @@ -1556,7 +1598,9 @@ static void eepro100_write4(EEPRO100State * s, uint32_t addr, uint32_t val)
>  static uint32_t ioport_read1(void *opaque, uint32_t addr)
>  {
>      EEPRO100State *s = opaque;
> -    //~ logout("addr=%s\n", regname(addr));
> +#if 0
> +    logout("addr=%s\n", regname(addr));
> +#endif
>      return eepro100_read1(s, addr - s->region[1]);
>  }
>  
> @@ -1575,7 +1619,9 @@ static uint32_t ioport_read4(void *opaque, uint32_t addr)
>  static void ioport_write1(void *opaque, uint32_t addr, uint32_t val)
>  {
>      EEPRO100State *s = opaque;
> -    //~ logout("addr=%s val=0x%02x\n", regname(addr), val);
> +#if 0
> +    logout("addr=%s val=0x%02x\n", regname(addr), val);
> +#endif
>      eepro100_write1(s, addr - s->region[1], val);
>  }
>  
> @@ -1623,42 +1669,54 @@ static void pci_map(PCIDevice * pci_dev, int region_num,
>  static void pci_mmio_writeb(void *opaque, target_phys_addr_t addr, uint32_t val)
>  {
>      EEPRO100State *s = opaque;
> -    //~ logout("addr=%s val=0x%02x\n", regname(addr), val);
> +#if 0
> +    logout("addr=%s val=0x%02x\n", regname(addr), val);
> +#endif
>      eepro100_write1(s, addr, val);
>  }
>  
>  static void pci_mmio_writew(void *opaque, target_phys_addr_t addr, uint32_t val)
>  {
>      EEPRO100State *s = opaque;
> -    //~ logout("addr=%s val=0x%02x\n", regname(addr), val);
> +#if 0
> +    logout("addr=%s val=0x%02x\n", regname(addr), val);
> +#endif
>      eepro100_write2(s, addr, val);
>  }
>  
>  static void pci_mmio_writel(void *opaque, target_phys_addr_t addr, uint32_t val)
>  {
>      EEPRO100State *s = opaque;
> -    //~ logout("addr=%s val=0x%02x\n", regname(addr), val);
> +#if 0
> +    logout("addr=%s val=0x%02x\n", regname(addr), val);
> +#endif
>      eepro100_write4(s, addr, val);
>  }
>  
>  static uint32_t pci_mmio_readb(void *opaque, target_phys_addr_t addr)
>  {
>      EEPRO100State *s = opaque;
> -    //~ logout("addr=%s\n", regname(addr));
> +#if 0
> +    logout("addr=%s\n", regname(addr));
> +#endif
>      return eepro100_read1(s, addr);
>  }
>  
>  static uint32_t pci_mmio_readw(void *opaque, target_phys_addr_t addr)
>  {
>      EEPRO100State *s = opaque;
> -    //~ logout("addr=%s\n", regname(addr));
> +#if 0
> +    logout("addr=%s\n", regname(addr));
> +#endif
>      return eepro100_read2(s, addr);
>  }
>  
>  static uint32_t pci_mmio_readl(void *opaque, target_phys_addr_t addr)
>  {
>      EEPRO100State *s = opaque;
> -    //~ logout("addr=%s\n", regname(addr));
> +#if 0
> +    logout("addr=%s\n", regname(addr));
> +#endif
>      return eepro100_read4(s, addr);
>  }
>  
> @@ -1695,7 +1753,9 @@ static int nic_can_receive(VLANClientState *nc)
>      EEPRO100State *s = DO_UPCAST(NICState, nc, nc)->opaque;
>      TRACE(RXTX, logout("%p\n", s));
>      return get_ru_state(s) == ru_ready;
> -    //~ return !eepro100_buffer_full(s);
> +#if 0
> +    return !eepro100_buffer_full(s);
> +#endif
>  }
>  
>  static ssize_t nic_receive(VLANClientState *nc, const uint8_t * buf, size_t size)
> @@ -1724,13 +1784,15 @@ static ssize_t nic_receive(VLANClientState *nc, const uint8_t * buf, size_t size
>           * Short frame is discarded */
>          logout("%p received short frame (%zu byte)\n", s, size);
>          s->statistics.rx_short_frame_errors++;
> -        //~ return -1;
> +#if 0
> +        return -1;
> +#endif
>      } else if ((size > MAX_ETH_FRAME_SIZE + 4) && !(s->configuration[18] & BIT(3))) {
>          /* Long frame and configuration byte 18/3 (long receive ok) not set:
>           * Long frames are discarded. */
>          logout("%p received long frame (%zu byte), ignored\n", s, size);
>          return -1;
> -    } else if (memcmp(buf, s->conf.macaddr.a, 6) == 0) {       // !!!
> +    } else if (memcmp(buf, s->conf.macaddr.a, 6) == 0) {       /* !!! */
>          /* Frame matches individual address. */
>          /* TODO: check configuration byte 15/4 (ignore U/L). */
>          TRACE(RXTX, logout("%p received frame for me, len=%zu\n", s, size));
> @@ -1774,11 +1836,12 @@ static ssize_t nic_receive(VLANClientState *nc, const uint8_t * buf, size_t size
>          /* TODO: RNR interrupt only at first failed frame? */
>          eepro100_rnr_interrupt(s);
>          s->statistics.rx_resource_errors++;
> -        //~ assert(!"no resources");
> +#if 0
> +        assert(!"no resources");
> +#endif
>          return -1;
>      }
> -    //~ !!!
> -//~ $3 = {status = 0x0, command = 0xc000, link = 0x2d220, rx_buf_addr = 0x207dc, count = 0x0, size = 0x5f8, packet = {0x0 <repeats 1518 times>}}
> +    /* !!! */
>      eepro100_rx_t rx;
>      cpu_physical_memory_read(s->ru_base + s->ru_offset, (uint8_t *) & rx,
>                               offsetof(eepro100_rx_t, packet));
> @@ -1799,14 +1862,18 @@ static ssize_t nic_receive(VLANClientState *nc, const uint8_t * buf, size_t size
>               rfd_status);
>      stw_phys(s->ru_base + s->ru_offset + offsetof(eepro100_rx_t, count), size);
>      /* Early receive interrupt not supported. */
> -    //~ eepro100_er_interrupt(s);
> +#if 0
> +    eepro100_er_interrupt(s);
> +#endif
>      /* Receive CRC Transfer not supported. */
>      if (s->configuration[18] & BIT(2)) {
>          missing("Receive CRC Transfer");
>          return -1;
>      }
>      /* TODO: check stripping enable bit. */
> -    //~ assert(!(s->configuration[17] & BIT(0)));
> +#if 0
> +    assert(!(s->configuration[17] & BIT(0)));
> +#endif
>      cpu_physical_memory_write(s->ru_base + s->ru_offset +
>                                offsetof(eepro100_rx_t, packet), buf, size);
>      s->statistics.rx_good_frames++;
> -- 
> 1.7.0
Stefan Weil - March 3, 2010, 7:15 p.m.
Michael S. Tsirkin schrieb:
> On Tue, Mar 02, 2010 at 10:37:59PM +0100, Stefan Weil wrote:
>> C++ comments are unwanted, so this is fixed here.
>>
>> * Replace C++ comments by C comments.
>> * Put code which was deactivated by a C++ comment in #if 0...#endif.
>>
>> Signed-off-by: Stefan Weil <weil@mail.berlios.de>
>
> It would be nice to add some documentation to commented
> blocks, explaining why we still keep them around
> and what happens if they are uncommented.


Typically, later patches either use this deactivated code (maybe after
some modifications), or they remove it completely. More documentation
would help of course - I'll try my best :-)

The purpose of this patch was only elimination of C++ comments by formally
transforming them to either C comments or by using the preprocessor #if.
Michael S. Tsirkin - March 3, 2010, 7:59 p.m.
On Wed, Mar 03, 2010 at 08:15:09PM +0100, Stefan Weil wrote:
> Michael S. Tsirkin schrieb:
> > On Tue, Mar 02, 2010 at 10:37:59PM +0100, Stefan Weil wrote:
> >> C++ comments are unwanted, so this is fixed here.
> >>
> >> * Replace C++ comments by C comments.
> >> * Put code which was deactivated by a C++ comment in #if 0...#endif.
> >>
> >> Signed-off-by: Stefan Weil <weil@mail.berlios.de>
> >
> > It would be nice to add some documentation to commented
> > blocks, explaining why we still keep them around
> > and what happens if they are uncommented.
> 
> 
> Typically, later patches either use this deactivated code (maybe after
> some modifications), or they remove it completely. More documentation
> would help of course - I'll try my best :-)
> 
> The purpose of this patch was only elimination of C++ comments by formally
> transforming them to either C comments or by using the preprocessor #if.

Yes, I understand that.

Patch

diff --git a/hw/eepro100.c b/hw/eepro100.c
index 0f07b70..43a9060 100644
--- a/hw/eepro100.c
+++ b/hw/eepro100.c
@@ -154,11 +154,13 @@  typedef struct {
     uint16_t tcb_bytes;         /* transmit command block byte count (in lower 14 bits */
     uint8_t tx_threshold;       /* transmit threshold */
     uint8_t tbd_count;          /* TBD number */
-    //~ /* This constitutes two "TBD" entries: hdr and data */
-    //~ uint32_t tx_buf_addr0;  /* void *, header of frame to be transmitted.  */
-    //~ int32_t  tx_buf_size0;  /* Length of Tx hdr. */
-    //~ uint32_t tx_buf_addr1;  /* void *, data to be transmitted.  */
-    //~ int32_t  tx_buf_size1;  /* Length of Tx data. */
+#if 0
+    /* This constitutes two "TBD" entries: hdr and data */
+    uint32_t tx_buf_addr0;  /* void *, header of frame to be transmitted.  */
+    int32_t  tx_buf_size0;  /* Length of Tx hdr. */
+    uint32_t tx_buf_addr1;  /* void *, data to be transmitted.  */
+    int32_t  tx_buf_size1;  /* Length of Tx data. */
+#endif
 } eepro100_tx_t;
 
 /* Receive frame descriptor. */
@@ -398,7 +400,9 @@  static void eepro100_interrupt(EEPRO100State * s, uint8_t status)
     s->mem[SCBAck] |= status;
     status = s->scb_stat = s->mem[SCBAck];
     status &= (mask | 0x0f);
-    //~ status &= (~s->mem[SCBIntmask] | 0x0xf);
+#if 0
+    status &= (~s->mem[SCBIntmask] | 0x0xf);
+#endif
     if (status && (mask & 0x01)) {
         /* SCB mask and SCB Bit M do not disable interrupt. */
         enable_interrupt(s);
@@ -477,9 +481,11 @@  static void pci_reset(EEPRO100State * s)
     pci_config_set_class(pci_conf, PCI_CLASS_NETWORK_ETHERNET);
     /* PCI Cache Line Size */
     /* check cache line size!!! */
-    //~ PCI_CONFIG_8(0x0c, 0x00);
+#if 0
+    PCI_CONFIG_8(0x0c, 0x00);
+#endif
     /* PCI Latency Timer */
-    PCI_CONFIG_8(PCI_LATENCY_TIMER, 0x20);   // latency timer = 32 clocks
+    PCI_CONFIG_8(PCI_LATENCY_TIMER, 0x20);   /* latency timer = 32 clocks */
     /* PCI Header Type */
     /* BIST (built-in self test) */
     /* Expansion ROM Base Address (depends on boot disable!!!) */
@@ -492,7 +498,7 @@  static void pci_reset(EEPRO100State * s)
     /* Interrupt Line */
     /* Interrupt Pin */
     /* TODO: RST# value should be 0 */
-    PCI_CONFIG_8(PCI_INTERRUPT_PIN, 1);      // interrupt pin 0
+    PCI_CONFIG_8(PCI_INTERRUPT_PIN, 1);      /* interrupt pin 0 */
     /* Minimum Grant */
     PCI_CONFIG_8(PCI_MIN_GNT, 0x08);
     /* Maximum Latency */
@@ -500,20 +506,20 @@  static void pci_reset(EEPRO100State * s)
 
     switch (device) {
     case i82550:
-        // TODO: check device id.
+        /* TODO: check device id. */
         pci_config_set_device_id(pci_conf, PCI_DEVICE_ID_INTEL_82551IT);
         /* Revision ID: 0x0c, 0x0d, 0x0e. */
         PCI_CONFIG_8(PCI_REVISION_ID, 0x0e);
-        // TODO: check size of statistical counters.
+        /* TODO: check size of statistical counters. */
         s->stats_size = 80;
-        // TODO: check extended tcb support.
+        /* TODO: check extended tcb support. */
         s->has_extended_tcb_support = 1;
         break;
     case i82551:
         pci_config_set_device_id(pci_conf, PCI_DEVICE_ID_INTEL_82551IT);
         /* Revision ID: 0x0f, 0x10. */
         PCI_CONFIG_8(PCI_REVISION_ID, 0x0f);
-        // TODO: check size of statistical counters.
+        /* TODO: check size of statistical counters. */
         s->stats_size = 80;
         s->has_extended_tcb_support = 1;
         break;
@@ -572,7 +578,7 @@  static void pci_reset(EEPRO100State * s)
         PCI_CONFIG_16(PCI_STATUS, PCI_STATUS_DEVSEL_MEDIUM |
                                   PCI_STATUS_FAST_BACK | PCI_STATUS_CAP_LIST);
         PCI_CONFIG_8(PCI_REVISION_ID, 0x08);
-        // TODO: Windows wants revision id 0x0c.
+        /* TODO: Windows wants revision id 0x0c. */
         PCI_CONFIG_8(PCI_REVISION_ID, 0x0c);
 #if EEPROM_SIZE > 0
         PCI_CONFIG_16(PCI_SUBSYSTEM_VENDOR_ID, 0x8086);
@@ -590,7 +596,7 @@  static void pci_reset(EEPRO100State * s)
         s->has_extended_tcb_support = 1;
         break;
     case i82562:
-        // TODO: check device id.
+        /* TODO: check device id. */
         pci_config_set_device_id(pci_conf, PCI_DEVICE_ID_INTEL_82551IT);
         /* TODO: wrong revision id. */
         PCI_CONFIG_8(PCI_REVISION_ID, 0x0e);
@@ -637,14 +643,16 @@  static void pci_reset(EEPRO100State * s)
 
 #if EEPROM_SIZE > 0
     if (device == i82557C || device == i82558B || device == i82559C) {
-        // TODO: get vendor id from EEPROM for i82557C or later.
-        // TODO: get device id from EEPROM for i82557C or later.
-        // TODO: status bit 4 can be disabled by EEPROM for i82558, i82559.
-        // TODO: header type is determined by EEPROM for i82559.
-        // TODO: get subsystem id from EEPROM for i82557C or later.
-        // TODO: get subsystem vendor id from EEPROM for i82557C or later.
-        // TODO: exp. rom baddr depends on a bit in EEPROM for i82558 or later.
-        // TODO: capability pointer depends on EEPROM for i82558.
+        /*
+        TODO: get vendor id from EEPROM for i82557C or later.
+        TODO: get device id from EEPROM for i82557C or later.
+        TODO: status bit 4 can be disabled by EEPROM for i82558, i82559.
+        TODO: header type is determined by EEPROM for i82559.
+        TODO: get subsystem id from EEPROM for i82557C or later.
+        TODO: get subsystem vendor id from EEPROM for i82557C or later.
+        TODO: exp. rom baddr depends on a bit in EEPROM for i82558 or later.
+        TODO: capability pointer depends on EEPROM for i82558.
+        */
         logout("Get device id and revision from EEPROM!!!\n");
     }
 #endif /* EEPROM_SIZE > 0 */
@@ -654,7 +662,9 @@  static void nic_selective_reset(EEPRO100State * s)
 {
     size_t i;
     uint16_t *eeprom_contents = eeprom93xx_data(s->eeprom);
-    //~ eeprom93xx_reset(s->eeprom);
+#if 0
+    eeprom93xx_reset(s->eeprom);
+#endif
     memcpy(eeprom_contents, s->conf.macaddr.a, 6);
     eeprom_contents[EEPROM_ID] = EEPROM_ID_VALID;
     if (s->device == i82557B || s->device == i82557C)
@@ -738,7 +748,7 @@  static void eepro100_write_status(EEPRO100State * s, uint16_t val)
 static uint16_t eepro100_read_command(EEPRO100State * s)
 {
     uint16_t val = 0xffff;
-    //~ TRACE(OTHER, logout("val=0x%04x\n", val));
+    TRACE(OTHER, logout("val=0x%04x\n", val));
     return val;
 }
 #endif
@@ -793,9 +803,11 @@  static void dump_statistics(EEPRO100State * s)
     stl_le_phys(s->statsaddr + 36, s->statistics.rx_good_frames);
     stl_le_phys(s->statsaddr + 48, s->statistics.rx_resource_errors);
     stl_le_phys(s->statsaddr + 60, s->statistics.rx_short_frame_errors);
-    //~ stw_le_phys(s->statsaddr + 76, s->statistics.xmt_tco_frames);
-    //~ stw_le_phys(s->statsaddr + 78, s->statistics.rcv_tco_frames);
-    //~ missing("CU dump statistical counters");
+#if 0
+    stw_le_phys(s->statsaddr + 76, s->statistics.xmt_tco_frames);
+    stw_le_phys(s->statsaddr + 78, s->statistics.rcv_tco_frames);
+    missing("CU dump statistical counters");
+#endif
 }
 
 static void read_cb(EEPRO100State *s)
@@ -832,7 +844,9 @@  static void tx_command(EEPRO100State *s)
     while (size < tcb_bytes) {
         uint32_t tx_buffer_address = ldl_phys(tbd_address);
         uint16_t tx_buffer_size = lduw_phys(tbd_address + 4);
-        //~ uint16_t tx_buffer_el = lduw_phys(tbd_address + 6);
+#if 0
+        uint16_t tx_buffer_el = lduw_phys(tbd_address + 6);
+#endif
         tbd_address += 8;
         TRACE(RXTX, logout
             ("TBD (simplified mode): buffer address 0x%08x, size 0x%04x\n",
@@ -889,7 +903,9 @@  static void tx_command(EEPRO100State *s)
     s->statistics.tx_good_frames++;
     /* Transmit with bad status would raise an CX/TNO interrupt.
      * (82557 only). Emulation never has bad status. */
-    //~ eepro100_cx_interrupt(s);
+#if 0
+    eepro100_cx_interrupt(s);
+#endif
 }
 
 static void set_multicast_list(EEPRO100State *s)
@@ -1016,7 +1032,9 @@  static void eepro100_cu_command(EEPRO100State * s, uint8_t val)
             logout("bad CU resume from CU state %u\n", get_cu_state(s));
             /* Workaround for bad Linux eepro100 driver which resumes
              * from idle state. */
-            //~ missing("cu resume");
+#if 0
+            missing("cu resume");
+#endif
             set_cu_state(s, cu_suspended);
         }
         if (get_cu_state(s) == cu_suspended) {
@@ -1067,7 +1085,9 @@  static void eepro100_ru_command(EEPRO100State * s, uint8_t val)
         /* RU start. */
         if (get_ru_state(s) != ru_idle) {
             logout("RU state is %u, should be %u\n", get_ru_state(s), ru_idle);
-            //~ assert(!"wrong RU state");
+#if 0
+            assert(!"wrong RU state");
+#endif
         }
         set_ru_state(s, ru_ready);
         s->ru_offset = s->pointer;
@@ -1078,7 +1098,9 @@  static void eepro100_ru_command(EEPRO100State * s, uint8_t val)
         if (get_ru_state(s) != ru_suspended) {
             logout("RU state is %u, should be %u\n", get_ru_state(s),
                    ru_suspended);
-            //~ assert(!"wrong RU state");
+#if 0
+            assert(!"wrong RU state");
+#endif
         }
         set_ru_state(s, ru_ready);
         break;
@@ -1140,7 +1162,9 @@  static void eepro100_write_eeprom(eeprom_t * eeprom, uint8_t val)
     TRACE(EEPROM, logout("val=0x%02x\n", val));
 
     /* mask unwriteable bits */
-    //~ val = SET_MASKED(val, 0x31, eeprom->value);
+#if 0
+    val = SET_MASKED(val, 0x31, eeprom->value);
+#endif
 
     int eecs = ((val & EEPROM_CS) != 0);
     int eesk = ((val & EEPROM_SK) != 0);
@@ -1222,7 +1246,9 @@  static void eepro100_write_mdi(EEPRO100State * s, uint32_t val)
           val, raiseint, mdi_op_name[opcode], phy, reg2name(reg), data));
     if (phy != 1) {
         /* Unsupported PHY address. */
-        //~ logout("phy must be 1 but is %u\n", phy);
+#if 0
+        logout("phy must be 1 but is %u\n", phy);
+#endif
         data = 0;
     } else if (opcode != 1 && opcode != 2) {
         /* Unsupported opcode. */
@@ -1368,16 +1394,22 @@  static uint8_t eepro100_read1(EEPRO100State * s, uint32_t addr)
 
     switch (addr) {
     case SCBStatus:
-        //~ val = eepro100_read_status(s);
+#if 0
+        val = eepro100_read_status(s);
+#endif
         TRACE(OTHER, logout("addr=%s val=0x%02x\n", regname(addr), val));
         break;
     case SCBAck:
-        //~ val = eepro100_read_status(s);
+#if 0
+        val = eepro100_read_status(s);
+#endif
         TRACE(OTHER, logout("addr=%s val=0x%02x\n", regname(addr), val));
         break;
     case SCBCmd:
         TRACE(OTHER, logout("addr=%s val=0x%02x\n", regname(addr), val));
-        //~ val = eepro100_read_command(s);
+#if 0
+        val = eepro100_read_command(s);
+#endif
         break;
     case SCBIntmask:
         TRACE(OTHER, logout("addr=%s val=0x%02x\n", regname(addr), val));
@@ -1413,7 +1445,9 @@  static uint16_t eepro100_read2(EEPRO100State * s, uint32_t addr)
 
     switch (addr) {
     case SCBStatus:
-        //~ val = eepro100_read_status(s);
+#if 0
+        val = eepro100_read_status(s);
+#endif
     case SCBCmd:
         TRACE(OTHER, logout("addr=%s val=0x%04x\n", regname(addr), val));
         break;
@@ -1437,11 +1471,15 @@  static uint32_t eepro100_read4(EEPRO100State * s, uint32_t addr)
 
     switch (addr) {
     case SCBStatus:
-        //~ val = eepro100_read_status(s);
+#if 0
+        val = eepro100_read_status(s);
+#endif
         TRACE(OTHER, logout("addr=%s val=0x%08x\n", regname(addr), val));
         break;
     case SCBPointer:
-        //~ val = eepro100_read_pointer(s);
+#if 0
+        val = eepro100_read_pointer(s);
+#endif
         TRACE(OTHER, logout("addr=%s val=0x%08x\n", regname(addr), val));
         break;
     case SCBPort:
@@ -1468,7 +1506,9 @@  static void eepro100_write1(EEPRO100State * s, uint32_t addr, uint8_t val)
 
     switch (addr) {
     case SCBStatus:
-        //~ eepro100_write_status(s, val);
+#if 0
+        eepro100_write_status(s, val);
+#endif
         break;
     case SCBAck:
         eepro100_acknowledge(s);
@@ -1508,7 +1548,9 @@  static void eepro100_write2(EEPRO100State * s, uint32_t addr, uint16_t val)
 
     switch (addr) {
     case SCBStatus:
-        //~ eepro100_write_status(s, val);
+#if 0
+        eepro100_write_status(s, val);
+#endif
         eepro100_acknowledge(s);
         break;
     case SCBCmd:
@@ -1556,7 +1598,9 @@  static void eepro100_write4(EEPRO100State * s, uint32_t addr, uint32_t val)
 static uint32_t ioport_read1(void *opaque, uint32_t addr)
 {
     EEPRO100State *s = opaque;
-    //~ logout("addr=%s\n", regname(addr));
+#if 0
+    logout("addr=%s\n", regname(addr));
+#endif
     return eepro100_read1(s, addr - s->region[1]);
 }
 
@@ -1575,7 +1619,9 @@  static uint32_t ioport_read4(void *opaque, uint32_t addr)
 static void ioport_write1(void *opaque, uint32_t addr, uint32_t val)
 {
     EEPRO100State *s = opaque;
-    //~ logout("addr=%s val=0x%02x\n", regname(addr), val);
+#if 0
+    logout("addr=%s val=0x%02x\n", regname(addr), val);
+#endif
     eepro100_write1(s, addr - s->region[1], val);
 }
 
@@ -1623,42 +1669,54 @@  static void pci_map(PCIDevice * pci_dev, int region_num,
 static void pci_mmio_writeb(void *opaque, target_phys_addr_t addr, uint32_t val)
 {
     EEPRO100State *s = opaque;
-    //~ logout("addr=%s val=0x%02x\n", regname(addr), val);
+#if 0
+    logout("addr=%s val=0x%02x\n", regname(addr), val);
+#endif
     eepro100_write1(s, addr, val);
 }
 
 static void pci_mmio_writew(void *opaque, target_phys_addr_t addr, uint32_t val)
 {
     EEPRO100State *s = opaque;
-    //~ logout("addr=%s val=0x%02x\n", regname(addr), val);
+#if 0
+    logout("addr=%s val=0x%02x\n", regname(addr), val);
+#endif
     eepro100_write2(s, addr, val);
 }
 
 static void pci_mmio_writel(void *opaque, target_phys_addr_t addr, uint32_t val)
 {
     EEPRO100State *s = opaque;
-    //~ logout("addr=%s val=0x%02x\n", regname(addr), val);
+#if 0
+    logout("addr=%s val=0x%02x\n", regname(addr), val);
+#endif
     eepro100_write4(s, addr, val);
 }
 
 static uint32_t pci_mmio_readb(void *opaque, target_phys_addr_t addr)
 {
     EEPRO100State *s = opaque;
-    //~ logout("addr=%s\n", regname(addr));
+#if 0
+    logout("addr=%s\n", regname(addr));
+#endif
     return eepro100_read1(s, addr);
 }
 
 static uint32_t pci_mmio_readw(void *opaque, target_phys_addr_t addr)
 {
     EEPRO100State *s = opaque;
-    //~ logout("addr=%s\n", regname(addr));
+#if 0
+    logout("addr=%s\n", regname(addr));
+#endif
     return eepro100_read2(s, addr);
 }
 
 static uint32_t pci_mmio_readl(void *opaque, target_phys_addr_t addr)
 {
     EEPRO100State *s = opaque;
-    //~ logout("addr=%s\n", regname(addr));
+#if 0
+    logout("addr=%s\n", regname(addr));
+#endif
     return eepro100_read4(s, addr);
 }
 
@@ -1695,7 +1753,9 @@  static int nic_can_receive(VLANClientState *nc)
     EEPRO100State *s = DO_UPCAST(NICState, nc, nc)->opaque;
     TRACE(RXTX, logout("%p\n", s));
     return get_ru_state(s) == ru_ready;
-    //~ return !eepro100_buffer_full(s);
+#if 0
+    return !eepro100_buffer_full(s);
+#endif
 }
 
 static ssize_t nic_receive(VLANClientState *nc, const uint8_t * buf, size_t size)
@@ -1724,13 +1784,15 @@  static ssize_t nic_receive(VLANClientState *nc, const uint8_t * buf, size_t size
          * Short frame is discarded */
         logout("%p received short frame (%zu byte)\n", s, size);
         s->statistics.rx_short_frame_errors++;
-        //~ return -1;
+#if 0
+        return -1;
+#endif
     } else if ((size > MAX_ETH_FRAME_SIZE + 4) && !(s->configuration[18] & BIT(3))) {
         /* Long frame and configuration byte 18/3 (long receive ok) not set:
          * Long frames are discarded. */
         logout("%p received long frame (%zu byte), ignored\n", s, size);
         return -1;
-    } else if (memcmp(buf, s->conf.macaddr.a, 6) == 0) {       // !!!
+    } else if (memcmp(buf, s->conf.macaddr.a, 6) == 0) {       /* !!! */
         /* Frame matches individual address. */
         /* TODO: check configuration byte 15/4 (ignore U/L). */
         TRACE(RXTX, logout("%p received frame for me, len=%zu\n", s, size));
@@ -1774,11 +1836,12 @@  static ssize_t nic_receive(VLANClientState *nc, const uint8_t * buf, size_t size
         /* TODO: RNR interrupt only at first failed frame? */
         eepro100_rnr_interrupt(s);
         s->statistics.rx_resource_errors++;
-        //~ assert(!"no resources");
+#if 0
+        assert(!"no resources");
+#endif
         return -1;
     }
-    //~ !!!
-//~ $3 = {status = 0x0, command = 0xc000, link = 0x2d220, rx_buf_addr = 0x207dc, count = 0x0, size = 0x5f8, packet = {0x0 <repeats 1518 times>}}
+    /* !!! */
     eepro100_rx_t rx;
     cpu_physical_memory_read(s->ru_base + s->ru_offset, (uint8_t *) & rx,
                              offsetof(eepro100_rx_t, packet));
@@ -1799,14 +1862,18 @@  static ssize_t nic_receive(VLANClientState *nc, const uint8_t * buf, size_t size
              rfd_status);
     stw_phys(s->ru_base + s->ru_offset + offsetof(eepro100_rx_t, count), size);
     /* Early receive interrupt not supported. */
-    //~ eepro100_er_interrupt(s);
+#if 0
+    eepro100_er_interrupt(s);
+#endif
     /* Receive CRC Transfer not supported. */
     if (s->configuration[18] & BIT(2)) {
         missing("Receive CRC Transfer");
         return -1;
     }
     /* TODO: check stripping enable bit. */
-    //~ assert(!(s->configuration[17] & BIT(0)));
+#if 0
+    assert(!(s->configuration[17] & BIT(0)));
+#endif
     cpu_physical_memory_write(s->ru_base + s->ru_offset +
                               offsetof(eepro100_rx_t, packet), buf, size);
     s->statistics.rx_good_frames++;