[Libreoffice-commits] libvisio.git: 4 commits - src/lib

David Tardon dtardon at redhat.com
Sun Apr 23 07:45:08 UTC 2017


 src/lib/VDXParser.cpp     |    2 
 src/lib/VisioDocument.cpp |  241 +++++++++++++---------------------------------
 src/lib/libvisio_utils.h  |    5 
 3 files changed, 75 insertions(+), 173 deletions(-)

New commits:
commit f0020452ade5eb48fb6e2c68b4c781e3069f2686
Author: David Tardon <dtardon at redhat.com>
Date:   Sun Apr 23 09:36:07 2017 +0200

    simplify the check
    
    Change-Id: I3a1f9cde30105d6d2f6d1159f09c94a0a6ad56c0

diff --git a/src/lib/VisioDocument.cpp b/src/lib/VisioDocument.cpp
index bc6ee9b..e07bfa7 100644
--- a/src/lib/VisioDocument.cpp
+++ b/src/lib/VisioDocument.cpp
@@ -7,6 +7,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/.
  */
 
+#include <algorithm>
 #include <memory>
 #include <string>
 
@@ -24,58 +25,19 @@
 namespace
 {
 
-#define VISIO_MAGIC_LENGTH 21
-
 static bool checkVisioMagic(librevenge::RVNGInputStream *input)
 {
+  const unsigned char magic[] =
+  {
+    0x56, 0x69, 0x73, 0x69, 0x6f, 0x20, 0x28, 0x54, 0x4d, 0x29,
+    0x20, 0x44, 0x72, 0x61, 0x77, 0x69, 0x6e, 0x67, 0x0d, 0x0a,
+    0x0
+  };
   int startPosition = (int)input->tell();
   unsigned long numBytesRead = 0;
-  const unsigned char *buffer = input->read(VISIO_MAGIC_LENGTH, numBytesRead);
-  bool returnValue = true;
-  if (VISIO_MAGIC_LENGTH != numBytesRead)
-    returnValue = false;
-  else if (0x56 != buffer[0])
-    returnValue = false;
-  else if (0x69 != buffer[1])
-    returnValue = false;
-  else if (0x73 != buffer[2])
-    returnValue = false;
-  else if (0x69 != buffer[3])
-    returnValue = false;
-  else if (0x6f != buffer[4])
-    returnValue = false;
-  else if (0x20 != buffer[5])
-    returnValue = false;
-  else if (0x28 != buffer[6])
-    returnValue = false;
-  else if (0x54 != buffer[7])
-    returnValue = false;
-  else if (0x4d != buffer[8])
-    returnValue = false;
-  else if (0x29 != buffer[9])
-    returnValue = false;
-  else if (0x20 != buffer[10])
-    returnValue = false;
-  else if (0x44 != buffer[11])
-    returnValue = false;
-  else if (0x72 != buffer[12])
-    returnValue = false;
-  else if (0x61 != buffer[13])
-    returnValue = false;
-  else if (0x77 != buffer[14])
-    returnValue = false;
-  else if (0x69 != buffer[15])
-    returnValue = false;
-  else if (0x6e != buffer[16])
-    returnValue = false;
-  else if (0x67 != buffer[17])
-    returnValue = false;
-  else if (0x0d != buffer[18])
-    returnValue = false;
-  else if (0x0a != buffer[19])
-    returnValue = false;
-  else if (0x00 != buffer[20])
-    returnValue = false;
+  const unsigned char *buffer = input->read(VSD_NUM_ELEMENTS(magic), numBytesRead);
+  const bool returnValue = VSD_NUM_ELEMENTS(magic) == numBytesRead
+                           && std::equal(magic, magic + VSD_NUM_ELEMENTS(magic), buffer);
   input->seek(startPosition, librevenge::RVNG_SEEK_SET);
   return returnValue;
 }
commit 82366ddc1bb6a69e675f3660f0fe48d7dda12e6e
Author: David Tardon <dtardon at redhat.com>
Date:   Sun Apr 23 09:29:27 2017 +0200

    there's nothing throwing an exception in the try block
    
    Change-Id: I9caabef5080dd6bf3ef35346e33e401433e57d53

diff --git a/src/lib/VisioDocument.cpp b/src/lib/VisioDocument.cpp
index 6a00ead..bc6ee9b 100644
--- a/src/lib/VisioDocument.cpp
+++ b/src/lib/VisioDocument.cpp
@@ -29,63 +29,55 @@ namespace
 static bool checkVisioMagic(librevenge::RVNGInputStream *input)
 {
   int startPosition = (int)input->tell();
-  try
-  {
-    unsigned long numBytesRead = 0;
-    const unsigned char *buffer = input->read(VISIO_MAGIC_LENGTH, numBytesRead);
-    bool returnValue = true;
-    if (VISIO_MAGIC_LENGTH != numBytesRead)
-      returnValue = false;
-    else if (0x56 != buffer[0])
-      returnValue = false;
-    else if (0x69 != buffer[1])
-      returnValue = false;
-    else if (0x73 != buffer[2])
-      returnValue = false;
-    else if (0x69 != buffer[3])
-      returnValue = false;
-    else if (0x6f != buffer[4])
-      returnValue = false;
-    else if (0x20 != buffer[5])
-      returnValue = false;
-    else if (0x28 != buffer[6])
-      returnValue = false;
-    else if (0x54 != buffer[7])
-      returnValue = false;
-    else if (0x4d != buffer[8])
-      returnValue = false;
-    else if (0x29 != buffer[9])
-      returnValue = false;
-    else if (0x20 != buffer[10])
-      returnValue = false;
-    else if (0x44 != buffer[11])
-      returnValue = false;
-    else if (0x72 != buffer[12])
-      returnValue = false;
-    else if (0x61 != buffer[13])
-      returnValue = false;
-    else if (0x77 != buffer[14])
-      returnValue = false;
-    else if (0x69 != buffer[15])
-      returnValue = false;
-    else if (0x6e != buffer[16])
-      returnValue = false;
-    else if (0x67 != buffer[17])
-      returnValue = false;
-    else if (0x0d != buffer[18])
-      returnValue = false;
-    else if (0x0a != buffer[19])
-      returnValue = false;
-    else if (0x00 != buffer[20])
-      returnValue = false;
-    input->seek(startPosition, librevenge::RVNG_SEEK_SET);
-    return returnValue;
-  }
-  catch (...)
-  {
-    input->seek(startPosition, librevenge::RVNG_SEEK_SET);
-    return false;
-  }
+  unsigned long numBytesRead = 0;
+  const unsigned char *buffer = input->read(VISIO_MAGIC_LENGTH, numBytesRead);
+  bool returnValue = true;
+  if (VISIO_MAGIC_LENGTH != numBytesRead)
+    returnValue = false;
+  else if (0x56 != buffer[0])
+    returnValue = false;
+  else if (0x69 != buffer[1])
+    returnValue = false;
+  else if (0x73 != buffer[2])
+    returnValue = false;
+  else if (0x69 != buffer[3])
+    returnValue = false;
+  else if (0x6f != buffer[4])
+    returnValue = false;
+  else if (0x20 != buffer[5])
+    returnValue = false;
+  else if (0x28 != buffer[6])
+    returnValue = false;
+  else if (0x54 != buffer[7])
+    returnValue = false;
+  else if (0x4d != buffer[8])
+    returnValue = false;
+  else if (0x29 != buffer[9])
+    returnValue = false;
+  else if (0x20 != buffer[10])
+    returnValue = false;
+  else if (0x44 != buffer[11])
+    returnValue = false;
+  else if (0x72 != buffer[12])
+    returnValue = false;
+  else if (0x61 != buffer[13])
+    returnValue = false;
+  else if (0x77 != buffer[14])
+    returnValue = false;
+  else if (0x69 != buffer[15])
+    returnValue = false;
+  else if (0x6e != buffer[16])
+    returnValue = false;
+  else if (0x67 != buffer[17])
+    returnValue = false;
+  else if (0x0d != buffer[18])
+    returnValue = false;
+  else if (0x0a != buffer[19])
+    returnValue = false;
+  else if (0x00 != buffer[20])
+    returnValue = false;
+  input->seek(startPosition, librevenge::RVNG_SEEK_SET);
+  return returnValue;
 }
 
 static bool isBinaryVisioDocument(librevenge::RVNGInputStream *input)
commit 09a06c334933593259393748fc3d76d8869c4342
Author: David Tardon <dtardon at redhat.com>
Date:   Sun Apr 23 09:25:54 2017 +0200

    shorten code by using smart ptrs
    
    Change-Id: I4510fc441e8ff55cf86d7e0ac2856d630d190f23

diff --git a/src/lib/VisioDocument.cpp b/src/lib/VisioDocument.cpp
index ec8fcd7..6a00ead 100644
--- a/src/lib/VisioDocument.cpp
+++ b/src/lib/VisioDocument.cpp
@@ -90,148 +90,91 @@ static bool checkVisioMagic(librevenge::RVNGInputStream *input)
 
 static bool isBinaryVisioDocument(librevenge::RVNGInputStream *input)
 {
-  librevenge::RVNGInputStream *docStream = 0;
-  try
+  std::shared_ptr<librevenge::RVNGInputStream> docStream;
+  input->seek(0, librevenge::RVNG_SEEK_SET);
+  if (input->isStructured())
   {
     input->seek(0, librevenge::RVNG_SEEK_SET);
-    if (input->isStructured())
-    {
-      input->seek(0, librevenge::RVNG_SEEK_SET);
-      docStream = input->getSubStreamByName("VisioDocument");
-    }
-    if (!docStream)
-      docStream = input;
-
-    docStream->seek(0, librevenge::RVNG_SEEK_SET);
-    unsigned char version = 0;
-    if (checkVisioMagic(docStream))
-    {
-      docStream->seek(0x1A, librevenge::RVNG_SEEK_SET);
-      version = libvisio::readU8(docStream);
-    }
-    input->seek(0, librevenge::RVNG_SEEK_SET);
-    if (docStream && docStream != input)
-      delete docStream;
-    docStream = 0;
-
-    VSD_DEBUG_MSG(("VisioDocument: version %i\n", version));
-
-    // Versions 2k (6) and 2k3 (11)
-    if ((version >= 1 && version <= 6) || version == 11)
-    {
-      return true;
-    }
+    docStream.reset(input->getSubStreamByName("VisioDocument"));
   }
-  catch (...)
+  if (!docStream)
+    docStream.reset(input, libvisio::VSDDummyDeleter());
+
+  docStream->seek(0, librevenge::RVNG_SEEK_SET);
+  unsigned char version = 0;
+  if (checkVisioMagic(docStream.get()))
   {
-    if (docStream && docStream != input)
-      delete docStream;
-    return false;
+    docStream->seek(0x1A, librevenge::RVNG_SEEK_SET);
+    version = libvisio::readU8(docStream.get());
   }
+  input->seek(0, librevenge::RVNG_SEEK_SET);
 
-  return false;
+  VSD_DEBUG_MSG(("VisioDocument: version %i\n", version));
+
+  // Versions 2k (6) and 2k3 (11)
+  return ((version >= 1 && version <= 6) || version == 11);
 }
 
 static bool parseBinaryVisioDocument(librevenge::RVNGInputStream *input, librevenge::RVNGDrawingInterface *painter, bool isStencilExtraction)
 {
   VSD_DEBUG_MSG(("Parsing Binary Visio Document\n"));
   input->seek(0, librevenge::RVNG_SEEK_SET);
-  librevenge::RVNGInputStream *docStream = 0;
+  std::shared_ptr<librevenge::RVNGInputStream> docStream;
   if (input->isStructured())
-    docStream = input->getSubStreamByName("VisioDocument");
+    docStream.reset(input->getSubStreamByName("VisioDocument"));
   if (!docStream)
-    docStream = input;
+    docStream.reset(input, libvisio::VSDDummyDeleter());
 
   docStream->seek(0x1A, librevenge::RVNG_SEEK_SET);
 
-  libvisio::VSDParser *parser = 0;
-  try
-  {
-    unsigned char version = libvisio::readU8(docStream);
-    switch (version)
-    {
-    case 1:
-    case 2:
-    case 3:
-    case 4:
-    case 5:
-      parser = new libvisio::VSD5Parser(docStream, painter);
-      break;
-    case 6:
-      parser = new libvisio::VSD6Parser(docStream, painter);
-      break;
-    case 11:
-      parser = new libvisio::VSDParser(docStream, painter, input);
-      break;
-    default:
-      break;
-    }
-
-    bool retValue = false;
-    if (parser)
-    {
-      if (isStencilExtraction)
-        retValue = parser->extractStencils();
-      else if (!isStencilExtraction)
-        retValue = parser->parseMain();
-    }
-    else
-    {
-      if (docStream != input)
-        delete docStream;
-      return false;
-    }
-
-    delete parser;
-    if (docStream != input)
-      delete docStream;
+  std::unique_ptr<libvisio::VSDParser> parser;
 
-    return retValue;
-  }
-  catch (...)
+  unsigned char version = libvisio::readU8(docStream.get());
+  switch (version)
   {
-    delete parser;
-    if (docStream != input)
-      delete docStream;
+  case 1:
+  case 2:
+  case 3:
+  case 4:
+  case 5:
+    parser.reset(new libvisio::VSD5Parser(docStream.get(), painter));
+    break;
+  case 6:
+    parser.reset(new libvisio::VSD6Parser(docStream.get(), painter));
+    break;
+  case 11:
+    parser.reset(new libvisio::VSDParser(docStream.get(), painter, input));
+    break;
+  default:
+    break;
   }
 
-  return false;
+  if (isStencilExtraction)
+    return parser->extractStencils();
+  else
+    return parser->parseMain();
 }
 
 static bool isOpcVisioDocument(librevenge::RVNGInputStream *input)
 {
-  librevenge::RVNGInputStream *tmpInput = 0;
-  try
-  {
-    input->seek(0, librevenge::RVNG_SEEK_SET);
-    if (!input->isStructured())
-      return false;
-
-    tmpInput = input->getSubStreamByName("_rels/.rels");
-    if (!tmpInput)
-      return false;
+  input->seek(0, librevenge::RVNG_SEEK_SET);
+  if (!input->isStructured())
+    return false;
 
-    libvisio::VSDXRelationships rootRels(tmpInput);
-    delete tmpInput;
+  std::unique_ptr<librevenge::RVNGInputStream> tmpInput(input->getSubStreamByName("_rels/.rels"));
+  if (!tmpInput)
+    return false;
 
-    // Check whether the relationship points to a Visio document stream
-    const libvisio::VSDXRelationship *rel = rootRels.getRelationshipByType("http://schemas.microsoft.com/visio/2010/relationships/document");
-    if (!rel)
-      return false;
+  libvisio::VSDXRelationships rootRels(tmpInput.get());
 
-    // check whether the pointed Visio document stream exists in the document
-    tmpInput = input->getSubStreamByName(rel->getTarget().c_str());
-    if (!tmpInput)
-      return false;
-    delete tmpInput;
-    return true;
-  }
-  catch (...)
-  {
-    if (tmpInput)
-      delete tmpInput;
+  // Check whether the relationship points to a Visio document stream
+  const libvisio::VSDXRelationship *rel = rootRels.getRelationshipByType("http://schemas.microsoft.com/visio/2010/relationships/document");
+  if (!rel)
     return false;
-  }
+
+  // check whether the pointed Visio document stream exists in the document
+  tmpInput.reset(input->getSubStreamByName(rel->getTarget().c_str()));
+  return bool(tmpInput);
 }
 
 static bool parseOpcVisioDocument(librevenge::RVNGInputStream *input, librevenge::RVNGDrawingInterface *painter, bool isStencilExtraction)
diff --git a/src/lib/libvisio_utils.h b/src/lib/libvisio_utils.h
index ec576d6..773f5b0 100644
--- a/src/lib/libvisio_utils.h
+++ b/src/lib/libvisio_utils.h
@@ -49,6 +49,11 @@ namespace libvisio
 
 typedef std::shared_ptr<librevenge::RVNGInputStream> RVNGInputStreamPtr_t;
 
+struct VSDDummyDeleter
+{
+  void operator()(void *) {}
+};
+
 uint8_t readU8(librevenge::RVNGInputStream *input);
 uint16_t readU16(librevenge::RVNGInputStream *input);
 int16_t readS16(librevenge::RVNGInputStream *input);
commit 4cb1b65766178ebbaac67da7189cad1fe47fdc2d
Author: David Tardon <dtardon at redhat.com>
Date:   Sun Apr 23 09:07:38 2017 +0200

    ofz#1248 fix null ptr deref.
    
    Change-Id: I925070d1cad5b04eb02c5a6d095c83bbe88d3a80

diff --git a/src/lib/VDXParser.cpp b/src/lib/VDXParser.cpp
index ac8b44d..2a3304b 100644
--- a/src/lib/VDXParser.cpp
+++ b/src/lib/VDXParser.cpp
@@ -781,7 +781,7 @@ void libvisio::VDXParser::readPageProps(xmlTextReaderPtr reader)
   }
   while ((XML_PAGEPROPS != tokenId || XML_READER_TYPE_END_ELEMENT != tokenType) && 1 == ret && (!m_watcher || !m_watcher->isError()));
 
-  if (m_isStencilStarted)
+  if (m_isStencilStarted && m_currentStencil)
   {
     m_currentStencil->m_shadowOffsetX = shadowOffsetX;
     m_currentStencil->m_shadowOffsetY = shadowOffsetY;


More information about the Libreoffice-commits mailing list