[AGENT++] SNMP++ Implementation of Address Types

[Jens Engel]

Hi Jochen Katz,

I just looked over the implementation of the Address-related SnmpSyntax
types.
The current Copy constructor (CopyCtor) implementation of the
Address-related SnmpSyntax types violates
data hiding, seperation-of-concern.

The special CopyCtors of Address types replicate a lot of code and
initialize attributes from
3 classes in the Class Inheritance chain: ${My}Address, Address,
SnmpSyntax.

BETTER:
Most functionality could be implemented by providing a generic
implementation in a Address::CopyCtor.
Buffering, SnmpSyntax related attributes could be initialized/copied from
other object (passed to CopyCtor).
The specific Address classes need only be copy/initialize their own
attributes.

NOTE: SnmpSyntax currently provides no CopyCtor implementation due to its
ValueObject variants.
Therefore, the generic Address class performs SnmpSyntax init/copy duties.

1.  Most code could be avoided if a CopyCtor delegation would be used.
2. Each class initializes/copies only its attribute values (if possible).
3. Attribute output_buffer should be moved to generic Address base class!?!

The current implementation, looks like:

//-----[ IP Address copy constructor ]---------------------------------
IpAddress::IpAddress(const IpAddress& other)
    : iv_friendly_name_status(0), ip_version(ipaddr.ip_version)
{
  ADDRESS_TRACE;

  // always initialize what type this object is
  smival.syntax = sNMP_SYNTAX_IPADDR;
  smival.value.string.len = other.smival.value.string.len;
  smival.value.string.ptr = address_buffer;

  memset(iv_friendly_name, 0, sizeof(char) * MAX_FRIENDLY_NAME);
  valid_flag = other.valid_flag;
  // If Address::CopyCtor() is provided -> GENERIC functionality.
  if (valid_flag)
  {
    // copy the address data
    MEMCPY(address_buffer, other.address_buffer, smival.value.string.len);
    // and the friendly name
    strcpy(iv_friendly_name, other.iv_friendly_name);
    if (!other.addr_changed)
    {
      memcpy(output_buffer, other.output_buffer, sizeof(unsigned char) *
OUTBUFF);
      addr_changed = false;
    }
  }
}

NEW, BETTER (with output_buffer attribute moved to Address base class):

Address::Address(const Address& other)
    : SnmpSyntax(other),      //< Should already copy smival.syntax
      valid_flag(other.valid_flag),
      addr_changed(other.addr_changed)
{
    // -- GENERIC ALGORITHM: Copy/Init from other object.
    smival.syntax = other.smival.syntax;
    smival.value.string.len = other.smival.value.string.len;
    smival.value.string.ptr = this->address_buffer;
    if (this->valid_flag) {
        // copy the address data
        MEMCPY(address_buffer, other.address_buffer,
smival.value.string.len);
        if (!other.addr_changed) {
            memcpy(output_buffer, other.output_buffer, sizeof(unsigned
char) * OUTBUFF);
            // POSTCONDITION(addr_changed == false); ALREAY-DONE
        }
    }
}

IpAddress::IpAddress(const IpAddress& other)
    : Address(other),
      iv_friendly_name_status(0), ip_version(other.ip_version)
{
    // PRECONDITION(other.smival.syntax == sNMP_SYNTAX_IPADDR);
    memset(iv_friendly_name, 0, sizeof(char) * MAX_FRIENDLY_NAME);
    if (this->valid_flag)
    {
        strcpy(iv_friendly_name, other.iv_friendly_name);
    }
    // POSTCONDITION(smival.syntax == sNMP_SYNTAX_IPADDR);
    // POSTCONDITION(smival.value.string.len ==
other.smival.value.string.len);
}

Ciao,
Jens Engel