← Back to team overview

mudlet-makers team mailing list archive

[Bug 1471406] [NEW] Insuffienctly well defined object types could theoretically at least cause problems

 

Public bug reported:

In a forum post inquiring about the maximum number of room IDs that
could be used {http://forums.mudlet.org/viewtopic.php?f=13&t=4825} I
realised that the TRoom::roomId member is specified as an "int" but as
far as I understand it as a signed integer (using 16 bits in total) is
only guaranteed to have a maximum positive value of 2^16-1 or 32767 -
now in practice it is likely to be using 4 bytes but it need not be so.

I would suggest that for this variable - and indeed *for* *all* *simple* *numeric* *types* *that* *get* *stored* *in* *a* *file* we define precisely the size of the object using the Qt provided signed/unsigned types:
quint16, qint16
quint32, qint32
quint64, qint64

For the map file I suggest the next time we revise the format we change
to read/write the above types, and downgrade to reading generic "ints"
etc. from old file formats.

For the Mudlet profile I note that we write a version 1.0 into the start
BUT WE HAVE COMMENTED OUT THE CHECK FOR THIS ON READING IN EXISTING CODE
- that needs to be reimplemented ASAP to minimise "current" code getting
broken when we do fix a few items in the future (e.g. the command
"seperater"/"separator" element cock-up) as the existing code in the
field will break or at least spew out "not understood element" debug
messages on future "new/improved/fixed" saved files.

Also, in regard to char types (Qt offers: quint8 & qint8): whilst the
char type is definitely 8 bits I note especially in the cTelnet class
some cases where it is not entire clear whether it is used as a signed
or unsigned object - as used it suggests signed but when e.g. we use
constants for Telnet options that are more than 127 in value - this
implies unsigned values may be put into plain "char" variables.

The Qt documents themselves also note:
"
QDataStream's binary format has evolved since Qt 1.0, and is likely to continue evolving to reflect changes done in Qt. When inputting or outputting complex types, it's very important to make sure that the same version of the stream (version()) is used for reading and writing. If you need both forward and backward compatibility, you can hardcode the version number in the application:

stream.setVersion(QDataStream::Qt_4_0);
"

We do not currently do this, so, theoretically if someone tries to load
a map file format version 4 (the earliest that we still support) there
is no guarantee that the current Qt libraries will parse the serialized
bytes in the same manner as they did in the past - however by addressing
that, this become an issue that we can solve for the future.

** Affects: mudlet
     Importance: Undecided
         Status: New

-- 
You received this bug notification because you are a member of Mudlet
Makers, which is subscribed to Mudlet.
https://bugs.launchpad.net/bugs/1471406

Title:
  Insuffienctly well defined object types could theoretically at least
  cause problems

Status in Mudlet the MUD client:
  New

Bug description:
  In a forum post inquiring about the maximum number of room IDs that
  could be used {http://forums.mudlet.org/viewtopic.php?f=13&t=4825} I
  realised that the TRoom::roomId member is specified as an "int" but as
  far as I understand it as a signed integer (using 16 bits in total) is
  only guaranteed to have a maximum positive value of 2^16-1 or 32767 -
  now in practice it is likely to be using 4 bytes but it need not be
  so.

  I would suggest that for this variable - and indeed *for* *all* *simple* *numeric* *types* *that* *get* *stored* *in* *a* *file* we define precisely the size of the object using the Qt provided signed/unsigned types:
  quint16, qint16
  quint32, qint32
  quint64, qint64

  For the map file I suggest the next time we revise the format we
  change to read/write the above types, and downgrade to reading generic
  "ints" etc. from old file formats.

  For the Mudlet profile I note that we write a version 1.0 into the
  start BUT WE HAVE COMMENTED OUT THE CHECK FOR THIS ON READING IN
  EXISTING CODE - that needs to be reimplemented ASAP to minimise
  "current" code getting broken when we do fix a few items in the future
  (e.g. the command "seperater"/"separator" element cock-up) as the
  existing code in the field will break or at least spew out "not
  understood element" debug messages on future "new/improved/fixed"
  saved files.

  Also, in regard to char types (Qt offers: quint8 & qint8): whilst the
  char type is definitely 8 bits I note especially in the cTelnet class
  some cases where it is not entire clear whether it is used as a signed
  or unsigned object - as used it suggests signed but when e.g. we use
  constants for Telnet options that are more than 127 in value - this
  implies unsigned values may be put into plain "char" variables.

  The Qt documents themselves also note:
  "
  QDataStream's binary format has evolved since Qt 1.0, and is likely to continue evolving to reflect changes done in Qt. When inputting or outputting complex types, it's very important to make sure that the same version of the stream (version()) is used for reading and writing. If you need both forward and backward compatibility, you can hardcode the version number in the application:

  stream.setVersion(QDataStream::Qt_4_0);
  "

  We do not currently do this, so, theoretically if someone tries to
  load a map file format version 4 (the earliest that we still support)
  there is no guarantee that the current Qt libraries will parse the
  serialized bytes in the same manner as they did in the past - however
  by addressing that, this become an issue that we can solve for the
  future.

To manage notifications about this bug go to:
https://bugs.launchpad.net/mudlet/+bug/1471406/+subscriptions


Follow ups

References