On Friday 19 May 2006 09:02, Jon Keating wrote:
> The TLV functions are specific only to ICQ, so in the end it will be a
> part of the ICQ plugin. So, I was thinking that it would be best to
> have the TLV functions be a part of a new class CIcqBuffer, which
> inherits from CBuffer or uses CBuffer as a member, if it provides a
> public interface for those functions (So, inheritance is better I
> think).
I actually started with a CIcqBuffer that inherited CBuffer. But while
implementing it, it didn't feel right (reasons below). So I made it aggregate
CBuffer instead. This meant that its only purpose was to handle TLVs, so
naming it CTlvBuffer felt more natural. No need to make it too ICQ specific
if it isn't necessary, even if ICQ will be the only user of the class (for
now at least).
If you take a look at the old CBuffer::readTLV() you'll see a function with
the purpose of "parsing" the buffer and creating an internal copy where the
buffer is split by TLV type. So, after calling this function you'll have two
copies of the buffer. And more importantly, you have transformed CBuffer from
being a simple class with one purpose, to a more complex one with two
independent purposes: being a buffer and a TLV buffer.
My proposal replaces readTLV with CTlvBuffer::load(CBuffer&) which parses the
argument, extracting the TLVs. They can then be read by the read functions.
When you wish to send TLVs, you create a CTlvBuffer, write the TLVs to it with
the write functions and then append it to a CBuffer with operator<<.
For the (admitly few) use-cases I've looked at, this approach seems to work at
least as well as the old. But, I might have missed some corner cases.
Some code from icqd-srv.cpp (with the old CBuffer)
nUin = packet.UnpackUinString();
nLevel = packet.UnpackUnsignedShortBE();
nUserClass = packet.UnpackUnsignedShortBE();
if (!packet.readTLV()) { /* Log error */ }
if (packet.getTLVLen(0x0006) == 4)
m_nDesiredStatus = packet.UnpackUnsignedLongTLV(0x0006);
if (packet.getTLVLen(0x000a) == 4) { ... }
With the new CBuffer and CTlvBuffer this would be:
packet >> nUin >> nLevel >> nUserClass;
CTlvBuffer tlv;
if (!tlv.load(packet)) { /* Log error */ }
if (tlv.tlvLength(0x0006) == 4)
m_nDesiredStatus = tlv.read32(0x0006);
if (tlv.tlvLength(0x000a) == 4) { ... }
Comments?
Additional, I will rename all read functions to unpack and all write functions
to pack, in the same manner as the old class. I didn't do this earlier since
I was interpreting pack as compress (packa in Swedish) which is something
they don't do. But I have since realised that it should be interpreted more
as wrap.
And read/write are really bad names. While writing CTlvBuffer I had to
constantly check the documentation to know if I were reading to or from the
buffer. Can't be a good sign ;)
// Erik
--
This message has been ROT-13 encrypted twice for extra security.
Erik Johansson
http://ejohansson.se
-------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
Licq-devel mailing list
[EMAIL PROTECTED]
https://lists.sourceforge.net/lists/listinfo/licq-devel
