Merge from trunk

(bzr r14059.2.11)

15 files changed, 115 insertions, 105 deletions

diff --git a/src/extension/internal/bitmap/imagemagick.cpp b/src/extension/internal/bitmap/imagemagick.cpp
index 76f35415e..bc0dd8e33 100644
--- a/src/extension/internal/bitmap/imagemagick.cpp
+++ b/src/extension/internal/bitmap/imagemagick.cpp
@@ -12,10 +12,6 @@
 # include "config.h"
 #endif
 
-#if GLIBMM_DISABLE_DEPRECATED && HAVE_GLIBMM_THREADS_H
-#include <glibmm/threads.h>
-#endif
-
 #include <libintl.h>
 
 #include <gtkmm/box.h>
@@ -70,8 +66,8 @@ ImageMagickDocCache::ImageMagickDocCache(Inkscape::UI::View::View * view) :
     _imageItems(NULL)
 {
     SPDesktop *desktop = (SPDesktop*)view;
-    const GSList *selectedItemList = desktop->selection->itemList();
-    int selectCount = g_slist_length((GSList *)selectedItemList);
+    const std::vector<SPItem*> selectedItemList = desktop->selection->itemList();
+    int selectCount = selectedItemList.size();
     
     // Init the data-holders
     _nodes = new Inkscape::XML::Node*[selectCount];
@@ -83,9 +79,8 @@ ImageMagickDocCache::ImageMagickDocCache(Inkscape::UI::View::View * view) :
     _imageItems = new SPItem*[selectCount];
 
     // Loop through selected items
-    for (; selectedItemList != NULL; selectedItemList = g_slist_next(selectedItemList))
-    {
-        SPItem *item = SP_ITEM(selectedItemList->data);
+    for (std::vector<SPItem*>::const_iterator i = selectedItemList.begin(); i != selectedItemList.end(); i++) {
+        SPItem *item = *i;
         Inkscape::XML::Node *node = reinterpret_cast<Inkscape::XML::Node *>(item->getRepr());
         if (!strcmp(node->name(), "image") || !strcmp(node->name(), "svg:image"))
         {
@@ -241,13 +236,10 @@ ImageMagick::prefs_effect(Inkscape::Extension::Effect *module, Inkscape::UI::Vie
 {
     SPDocument * current_document = view->doc();
 
-    using Inkscape::Util::GSListConstIterator;
-
-    // FIXME very unsafe cast
-    GSListConstIterator<SPItem *> selected = ((SPDesktop *)view)->getSelection()->itemList();
+    std::vector<SPItem*> selected = ((SPDesktop *)view)->getSelection()->itemList();
     Inkscape::XML::Node * first_select = NULL;
-    if (selected != NULL) {
-        first_select = (*selected)->getRepr();
+    if (!selected.empty()) {
+        first_select = (selected.front())->getRepr();
     }
 
     return module->autogui(current_document, first_select, changeSignal);
diff --git a/src/extension/internal/bluredge.cpp b/src/extension/internal/bluredge.cpp
index 3ce537d9f..9f19f8b3b 100644
--- a/src/extension/internal/bluredge.cpp
+++ b/src/extension/internal/bluredge.cpp
@@ -63,13 +63,11 @@ BlurEdge::effect (Inkscape::Extension::Effect *module, Inkscape::UI::View::View
     Inkscape::Preferences *prefs = Inkscape::Preferences::get();
     double old_offset = prefs->getDouble("/options/defaultoffsetwidth/value", 1.0, "px");
 
-    using Inkscape::Util::GSListConstIterator;
     // TODO need to properly refcount the items, at least
-    std::list<SPItem *> items;
-    items.insert<GSListConstIterator<SPItem *> >(items.end(), selection->itemList(), NULL);
+    std::vector<SPItem*> items(selection->itemList());
     selection->clear();
 
-    for(std::list<SPItem *>::iterator item = items.begin();
+    for(std::vector<SPItem*>::iterator item = items.begin();
             item != items.end(); ++item) {
         SPItem * spitem = *item;
 
diff --git a/src/extension/internal/cairo-render-context.cpp b/src/extension/internal/cairo-render-context.cpp
index 2d6619e1e..27e34dbcf 100644
--- a/src/extension/internal/cairo-render-context.cpp
+++ b/src/extension/internal/cairo-render-context.cpp
@@ -1393,6 +1393,7 @@ CairoRenderContext::_setStrokeStyle(SPStyle const *style, Geom::OptRect const &p
             dashes[i] = style->stroke_dasharray.values[i];
         }
         cairo_set_dash(_cr, dashes, ndashes, style->stroke_dashoffset.value);
+        free(dashes);
     } else {
         cairo_set_dash(_cr, NULL, 0, 0.0);  // disable dashing
     }
diff --git a/src/extension/internal/cairo-renderer.cpp b/src/extension/internal/cairo-renderer.cpp
index 7fbdc4296..5a5553e97 100644
--- a/src/extension/internal/cairo-renderer.cpp
+++ b/src/extension/internal/cairo-renderer.cpp
@@ -294,14 +294,12 @@ static void sp_group_render(SPGroup *group, CairoRenderContext *ctx)
     CairoRenderer *renderer = ctx->getRenderer();
     TRACE(("sp_group_render opacity: %f\n", SP_SCALE24_TO_FLOAT(item->style->opacity.value)));
 
-    GSList *l = g_slist_reverse(group->childList(false));
-    while (l) {
-        SPObject *o = reinterpret_cast<SPObject *>(l->data);
-        SPItem *item = dynamic_cast<SPItem *>(o);
+    std::vector<SPObject*> l(group->childList(false));
+    for(std::vector<SPObject*>::const_iterator x = l.begin(); x!= l.end(); x++){
+        SPItem *item = dynamic_cast<SPItem*>(*x);
         if (item) {
             renderer->renderItem(ctx, item);
         }
-        l = g_slist_remove (l, o);
     }
 }
 
diff --git a/src/extension/internal/cdr-input.h b/src/extension/internal/cdr-input.h
index 3de6c1ed0..10af41d5a 100644
--- a/src/extension/internal/cdr-input.h
+++ b/src/extension/internal/cdr-input.h
@@ -19,10 +19,6 @@
 
 #ifdef WITH_LIBCDR
 
-#if GLIBMM_DISABLE_DEPRECATED && HAVE_GLIBMM_THREADS_H
-#include <glibmm/threads.h>
-#endif
-
 #include <gtkmm/dialog.h>
 
 #include "../implementation/implementation.h"
diff --git a/src/extension/internal/emf-inout.cpp b/src/extension/internal/emf-inout.cpp
index 31e69706f..e88cf3d42 100644
--- a/src/extension/internal/emf-inout.cpp
+++ b/src/extension/internal/emf-inout.cpp
@@ -480,7 +480,7 @@ uint32_t Emf::add_image(PEMF_CALLBACK_DATA d,  void *pEmr, uint32_t cbBits, uint
     uint32_t width, height, colortype, numCt, invert; // if needed these values will be set in get_DIB_params
     if(cbBits && cbBmi  && (iUsage == U_DIB_RGB_COLORS)){
         // next call returns pointers and values, but allocates no memory
-        dibparams = get_DIB_params(pEmr, offBits, offBmi, &px, (const U_RGBQUAD **) &ct,
+        dibparams = get_DIB_params((const char *)pEmr, offBits, offBmi, &px, (const U_RGBQUAD **) &ct,
             &numCt, &width, &height, &colortype, &invert);
         if(dibparams ==U_BI_RGB){
             // U_EMRCREATEMONOBRUSH uses text/bk colors instead of what is in the color map.
@@ -1307,7 +1307,7 @@ Emf::select_extpen(PEMF_CALLBACK_DATA d, int index)
             d->dc[d->level].stroke_set    = true;
         }
         else if(pEmr->elp.elpBrushStyle == U_BS_DIBPATTERN || pEmr->elp.elpBrushStyle == U_BS_DIBPATTERNPT){
-            d->dc[d->level].stroke_idx  = add_image(d, pEmr, pEmr->cbBits, pEmr->cbBmi, *(uint32_t *) &(pEmr->elp.elpColor), pEmr->offBits, pEmr->offBmi);
+            d->dc[d->level].stroke_idx  = add_image(d, (void *)pEmr, pEmr->cbBits, pEmr->cbBmi, *(uint32_t *) &(pEmr->elp.elpColor), pEmr->offBits, pEmr->offBmi);
             d->dc[d->level].stroke_mode = DRAW_IMAGE;
             d->dc[d->level].stroke_set  = true;
         }
@@ -1530,7 +1530,7 @@ void Emf::common_image_extraction(PEMF_CALLBACK_DATA d, void *pEmr,
     uint32_t width, height, colortype, numCt, invert; // if needed these values will be set in get_DIB_params
     if(cbBits && cbBmi && (iUsage == U_DIB_RGB_COLORS)){
         // next call returns pointers and values, but allocates no memory
-        dibparams = get_DIB_params(pEmr, offBits, offBmi, &px, (const U_RGBQUAD **) &ct,
+        dibparams = get_DIB_params((const char *)pEmr, offBits, offBmi, &px, (const U_RGBQUAD **) &ct,
             &numCt, &width, &height, &colortype, &invert);
         if(dibparams ==U_BI_RGB){
             if(sw == 0 || sh == 0){
@@ -1611,6 +1611,10 @@ int Emf::myEnhMetaFileProc(char *contents, unsigned int length, PEMF_CALLBACK_DA
     uint32_t         off=0;
     uint32_t         emr_mask;
     int              OK =1;
+    int              file_status=1;
+    uint32_t         nSize;
+    uint32_t         iType;
+    const char      *blimit = contents + length;
     PU_ENHMETARECORD lpEMFR;
     TCHUNK_SPECS     tsp;
     uint32_t         tbkMode  = U_TRANSPARENT;          // holds proposed change to bkMode, if text is involved saving these to the DC must wait until the text is written
@@ -1641,19 +1645,32 @@ int Emf::myEnhMetaFileProc(char *contents, unsigned int length, PEMF_CALLBACK_DA
     while(OK){
     if(off>=length)return(0);  //normally should exit from while after EMREOF sets OK to false.
 
+    // check record sizes and types thoroughly 
+    int badrec = 0;
+    if (!U_emf_record_sizeok(contents + off, blimit, &nSize, &iType, 1) || 
+        !U_emf_record_safe(contents + off)){
+        badrec = 1;
+    }
+    else {
+        emr_mask = emr_properties(iType);
+        if (emr_mask == U_EMR_INVALID) { badrec = 1; }
+    }
+    if (badrec) {
+        file_status = 0;
+        break;
+    }
+
     lpEMFR = (PU_ENHMETARECORD)(contents + off);
+
 //  Uncomment the following to track down toxic records
-// std::cout << "record type: " << lpEMFR->iType  << " name " << U_emr_names(lpEMFR->iType) << " length: " << lpEMFR->nSize << " offset: " << off <<std::endl;
-    off += lpEMFR->nSize;
+// std::cout << "record type: " << iType  << " name " << U_emr_names(iType) << " length: " << nSize << " offset: " << off <<std::endl;
+    off += nSize;
 
     SVGOStringStream tmp_outsvg;
     SVGOStringStream tmp_path;
     SVGOStringStream tmp_str;
     SVGOStringStream dbg_str;
 
-    emr_mask = emr_properties(lpEMFR->iType);
-    if(emr_mask == U_EMR_INVALID){ throw "Inkscape fatal memory allocation error - cannot continue"; }
-
 /* Uncomment the following to track down text problems */
 //std::cout << "tri->dirty:"<< d->tri->dirty << " emr_mask: " << std::hex << emr_mask << std::dec << std::endl;
 
@@ -1753,7 +1770,7 @@ std::cout << "BEFORE DRAW"
     }
 // std::cout << "AFTER DRAW logic d->mask: " << std::hex << d->mask << " emr_mask: " << emr_mask << std::dec << std::endl;
 
-    switch (lpEMFR->iType)
+    switch (iType)
     {
         case U_EMR_HEADER:
         {
@@ -3471,7 +3488,7 @@ std::cout << "BEFORE DRAW"
 // std::cout << d->outsvg << std::endl;
     (void) emr_properties(U_EMR_INVALID);  // force the release of the lookup table memory, returned value is irrelevant
 
-    return 1;
+    return(file_status);
 }
 
 void Emf::free_emf_strings(EMF_STRINGS name){
@@ -3486,12 +3503,14 @@ void Emf::free_emf_strings(EMF_STRINGS name){
 SPDocument *
 Emf::open( Inkscape::Extension::Input * /*mod*/, const gchar *uri )
 {
-    EMF_CALLBACK_DATA d;
-
     if (uri == NULL) {
         return NULL;
     }
 
+    EMF_CALLBACK_DATA d;
+
+    d.n_obj   = 0;     //these might not be set otherwise if the input file is corrupt
+    d.emf_obj = NULL; 
     d.dc[0].font_name = strdup("Arial"); // Default font, set only on lowest level, it copies up from there EMF spec says device can pick whatever it wants
 
     // set up the size default for patterns in defs.  This might not be referenced if there are no patterns defined in the drawing.
@@ -3518,14 +3537,17 @@ Emf::open( Inkscape::Extension::Input * /*mod*/, const gchar *uri )
       FT_LOAD_NO_SCALE | FT_LOAD_NO_HINTING  | FT_LOAD_NO_BITMAP,
       FT_KERNING_UNSCALED);
 
-    (void) myEnhMetaFileProc(contents,length, &d);
+    int good = myEnhMetaFileProc(contents,length, &d);
     free(contents);
 
     if (d.pDesc){ free( d.pDesc ); }
 
 //    std::cout << "SVG Output: " << std::endl << d.outsvg << std::endl;
 
-    SPDocument *doc = SPDocument::createNewDocFromMem(d.outsvg.c_str(), strlen(d.outsvg.c_str()), TRUE);
+    SPDocument *doc = NULL;
+    if (good) {
+        doc = SPDocument::createNewDocFromMem(d.outsvg.c_str(), strlen(d.outsvg.c_str()), TRUE);
+    }
 
     free_emf_strings(d.hatches);
     free_emf_strings(d.images);
diff --git a/src/extension/internal/emf-inout.h b/src/extension/internal/emf-inout.h
index c64299093..dd1d8f9c4 100644
--- a/src/extension/internal/emf-inout.h
+++ b/src/extension/internal/emf-inout.h
@@ -13,6 +13,8 @@
 #define SEEN_EXTENSION_INTERNAL_EMF_H
 
 #include <libuemf/uemf.h>
+#include <libuemf/uemf_safe.h>
+#include <libuemf/uemf_endian.h> // for U_emf_record_sizeok()
 #include "extension/internal/metafile-inout.h" // picks up PNG
 #include "extension/implementation/implementation.h"
 #include "style.h"
diff --git a/src/extension/internal/emf-print.cpp b/src/extension/internal/emf-print.cpp
index 7815c51c7..7c514d6e7 100644
--- a/src/extension/internal/emf-print.cpp
+++ b/src/extension/internal/emf-print.cpp
@@ -60,6 +60,7 @@
 #include "splivarot.h"             // pieces for union on shapes
 #include "2geom/svg-path-parser.h" // to get from SVG text to Geom::Path
 #include "display/canvas-bpath.h"  // for SPWindRule
+#include "display/cairo-utils.h"  // for Inkscape::Pixbuf::PF_CAIRO
 
 #include "emf-print.h"
 
@@ -70,7 +71,6 @@ namespace Internal {
 
 #define PXPERMETER 2835
 
-
 /* globals */
 static double       PX2WORLD;
 static bool         FixPPTCharPos, FixPPTDashLine, FixPPTGrad2Polys, FixPPTLinGrad, FixPPTPatternAsHatch, FixImageRot;
@@ -481,9 +481,8 @@ int PrintEmf::create_brush(SPStyle const *style, PU_COLORREF fcolor)
         rgba_px = (char *) pixbuf->pixels(); // Do NOT free this!!!
         colortype = U_BCBM_COLOR32;
         (void) RGBA_to_DIB(&px, &cbPx, &ct, &numCt,  rgba_px,  width, height, width * 4, colortype, 0, 1);
-        // Not sure why the next swap is needed because the preceding does it, and the code is identical
-        // to that in stretchdibits_set, which does not need this.
-        swapRBinRGBA(px, width * height);
+        // pixbuf can be either PF_CAIRO or PF_GDK, and these have R and B bytes swapped
+        if (pixbuf->pixelFormat() == Inkscape::Pixbuf::PF_CAIRO) { swapRBinRGBA(px, width * height); }
         Bmih = bitmapinfoheader_set(width, height, 1, colortype, U_BI_RGB, 0, PXPERMETER, PXPERMETER, numCt, 0);
         Bmi = bitmapinfo_set(Bmih, ct);
         rec = createdibpatternbrushpt_set(&brush, eht, U_DIB_RGB_COLORS, Bmi, cbPx, px);
@@ -584,9 +583,8 @@ int PrintEmf::create_pen(SPStyle const *style, const Geom::Affine &transform)
                 rgba_px = (char *) pixbuf->pixels(); // Do NOT free this!!!
                 colortype = U_BCBM_COLOR32;
                 (void) RGBA_to_DIB(&px, &cbPx, &ct, &numCt,  rgba_px,  width, height, width * 4, colortype, 0, 1);
-                // Not sure why the next swap is needed because the preceding does it, and the code is identical
-                // to that in stretchdibits_set, which does not need this.
-                swapRBinRGBA(px, width * height);
+                // pixbuf can be either PF_CAIRO or PF_GDK, and these have R and B bytes swapped
+                if (pixbuf->pixelFormat() == Inkscape::Pixbuf::PF_CAIRO) { swapRBinRGBA(px, width * height); }
                 Bmih = bitmapinfoheader_set(width, height, 1, colortype, U_BI_RGB, 0, PXPERMETER, PXPERMETER, numCt, 0);
                 Bmi = bitmapinfo_set(Bmih, ct);
             } else { // pattern
diff --git a/src/extension/internal/filter/filter.cpp b/src/extension/internal/filter/filter.cpp
index a2c565699..65162af22 100644
--- a/src/extension/internal/filter/filter.cpp
+++ b/src/extension/internal/filter/filter.cpp
@@ -125,15 +125,13 @@ void Filter::effect(Inkscape::Extension::Effect *module, Inkscape::UI::View::Vie
 	//printf("Calling filter effect\n");
     Inkscape::Selection * selection = ((SPDesktop *)document)->selection;
 
-    using Inkscape::Util::GSListConstIterator;
     // TODO need to properly refcount the items, at least
-    std::list<SPItem *> items;
-    items.insert<GSListConstIterator<SPItem *> >(items.end(), selection->itemList(), NULL);
+    std::vector<SPItem*> items(selection->itemList());
 
 	Inkscape::XML::Document * xmldoc = document->doc()->getReprDoc();
 	Inkscape::XML::Node * defsrepr = document->doc()->getDefs()->getRepr();
 
-    for(std::list<SPItem *>::iterator item = items.begin();
+    for(std::vector<SPItem*>::iterator item = items.begin();
             item != items.end(); ++item) {
         SPItem * spitem = *item;
 	Inkscape::XML::Node * node = spitem->getRepr();
diff --git a/src/extension/internal/grid.cpp b/src/extension/internal/grid.cpp
index 270edfe44..8fd82b675 100644
--- a/src/extension/internal/grid.cpp
+++ b/src/extension/internal/grid.cpp
@@ -15,10 +15,6 @@
 # include <config.h>
 #endif
 
-#if GLIBMM_DISABLE_DEPRECATED && HAVE_GLIBMM_THREADS_H
-#include <glibmm/threads.h>
-#endif
-
 #include <gtkmm/box.h>
 #include <gtkmm/adjustment.h>
 #include <gtkmm/spinbutton.h>
@@ -190,13 +186,10 @@ Grid::prefs_effect(Inkscape::Extension::Effect *module, Inkscape::UI::View::View
 {
     SPDocument * current_document = view->doc();
 
-    using Inkscape::Util::GSListConstIterator;
-
-    // FIXME very unsafe cast
-    GSListConstIterator<SPItem *> selected = ((SPDesktop *)view)->getSelection()->itemList();
+    std::vector<SPItem*> selected = ((SPDesktop *)view)->getSelection()->itemList();
     Inkscape::XML::Node * first_select = NULL;
-    if (selected != NULL) {
-        first_select = (*selected)->getRepr();
+    if (!selected.empty()) {
+        first_select = selected[0]->getRepr();
     }
 
     return module->autogui(current_document, first_select, changeSignal);
diff --git a/src/extension/internal/latex-text-renderer.cpp b/src/extension/internal/latex-text-renderer.cpp
index ab0733848..1026f51ad 100644
--- a/src/extension/internal/latex-text-renderer.cpp
+++ b/src/extension/internal/latex-text-renderer.cpp
@@ -228,14 +228,12 @@ LaTeXTextRenderer::writePostamble()
 
 void LaTeXTextRenderer::sp_group_render(SPGroup *group)
 {
-    GSList *l = g_slist_reverse(group->childList(false));
-    while (l) {
-        SPObject *o = reinterpret_cast<SPObject *>(l->data);
-        SPItem *item = dynamic_cast<SPItem *>(o);
+	std::vector<SPObject*> l = (group->childList(false));
+    for(std::vector<SPObject*>::const_iterator x = l.begin(); x != l.end(); x++){
+        SPItem *item = dynamic_cast<SPItem*>(*x);
         if (item) {
             renderItem(item);
         }
-        l = g_slist_remove (l, o);
     }
 }
 
diff --git a/src/extension/internal/pdfinput/pdf-input.h b/src/extension/internal/pdfinput/pdf-input.h
index d57c3e993..866db5d82 100644
--- a/src/extension/internal/pdfinput/pdf-input.h
+++ b/src/extension/internal/pdfinput/pdf-input.h
@@ -16,10 +16,6 @@
 
 #ifdef HAVE_POPPLER
 
-#if GLIBMM_DISABLE_DEPRECATED && HAVE_GLIBMM_THREADS_H
-#include <glibmm/threads.h>
-#endif
-
 #include <gtkmm/dialog.h>
 
 #include "../../implementation/implementation.h"
diff --git a/src/extension/internal/vsd-input.h b/src/extension/internal/vsd-input.h
index 3414e0ec9..acc52debf 100644
--- a/src/extension/internal/vsd-input.h
+++ b/src/extension/internal/vsd-input.h
@@ -19,10 +19,6 @@
 
 #ifdef WITH_LIBVISIO
 
-#if GLIBMM_DISABLE_DEPRECATED && HAVE_GLIBMM_THREADS_H
-#include <glibmm/threads.h>
-#endif
-
 #include <gtkmm/dialog.h>
 
 #include "../implementation/implementation.h"
diff --git a/src/extension/internal/wmf-inout.cpp b/src/extension/internal/wmf-inout.cpp
index 503a93418..f76fa16b4 100644
--- a/src/extension/internal/wmf-inout.cpp
+++ b/src/extension/internal/wmf-inout.cpp
@@ -95,7 +95,6 @@ Wmf::print_document_to_file(SPDocument *doc, const gchar *filename)
     SPPrintContext context;
     const gchar *oldconst;
     gchar *oldoutput;
-    unsigned int ret;
 
     doc->ensureUpToDate();
 
@@ -114,13 +113,12 @@ Wmf::print_document_to_file(SPDocument *doc, const gchar *filename)
     mod->root = mod->base->invoke_show(drawing, mod->dkey, SP_ITEM_SHOW_DISPLAY);
     drawing.setRoot(mod->root);
     /* Print document */
-    ret = mod->begin(doc);
-    if (ret) {
+    if (mod->begin(doc)) {
         g_free(oldoutput);
         throw Inkscape::Extension::Output::save_failed();
     }
     mod->base->invoke_print(&context);
-    ret = mod->finish();
+    mod->finish();
     /* Release arena */
     mod->base->invoke_hide(mod->dkey);
     mod->base = NULL;
@@ -1497,6 +1495,7 @@ int Wmf::myMetaFileProc(const char *contents, unsigned int length, PWMF_CALLBACK
     uint32_t         off=0;
     uint32_t         wmr_mask;
     int              OK =1;
+    int              file_status=1;
     TCHUNK_SPECS     tsp;
     uint8_t          iType;
     int              nSize;   // size of the current record, in bytes, or an error value if <=0
@@ -1562,7 +1561,9 @@ int Wmf::myMetaFileProc(const char *contents, unsigned int length, PWMF_CALLBACK
         U_WMRHEADER Header;
         off = 0;
         nSize = wmfheader_get(contents, blimit, &Placeable, &Header);
-        if(!nSize)return(0);
+        if (!nSize) {
+            return(0);
+        }
         if(!Header.nObjects){  Header.nObjects = 256; }// there _may_ be WMF files with no objects, more likely it is corrupt.  Try to use it anyway.
         d->n_obj     = Header.nObjects;
         d->wmf_obj   = new WMF_OBJECT[d->n_obj];
@@ -1602,7 +1603,10 @@ int Wmf::myMetaFileProc(const char *contents, unsigned int length, PWMF_CALLBACK
                     else {
                         off += nSize;
                     }
-                  }
+                }
+                else {
+                    return(0);
+                }
             }
             off=0;
             nSize = hold_nSize;
@@ -1665,27 +1669,32 @@ int Wmf::myMetaFileProc(const char *contents, unsigned int length, PWMF_CALLBACK
 
 
     while(OK){
-    if(off>=length)return(0);  //normally should exit from while after WMREOF sets OK to false.
+    if (off>=length) {
+        return(0);  //normally should exit from while after WMREOF sets OK to false.
+    }
     contents += nSize;         // pointer to the start of the next record
     off      += nSize;         // offset from beginning of buffer to the start of the next record
 
-    SVGOStringStream tmp_path;
-    SVGOStringStream tmp_str;
-
-    /*  Check that the current record size is OK, abort if not.
-        Pointer math might wrap, so check both sides of the range.
-        Some of the records will reset this with the same value,others will not
-        return a value at this time. */
+    /*  Currently this is a weaker check than for EMF, it only checks the size of the constant part
+        of the record */
     nSize = U_WMRRECSAFE_get(contents, blimit);
-    if(!nSize)break;
+    if(!nSize) {
+        file_status = 0;
+        break;
+    }
 
     iType     = *(uint8_t *)(contents + offsetof(U_METARECORD, iType )  );
 
+    wmr_mask = U_wmr_properties(iType);
+    if (wmr_mask == U_WMR_INVALID) { 
+        file_status = 0;
+        break;
+    }
 //  Uncomment the following to track down toxic records
 // std::cout << "record type: " << (int) iType << " name " << U_wmr_names(iType) << " length: " << nSize << " offset: " << off <<std::endl;
 
-    wmr_mask = U_wmr_properties(iType);
-    if(wmr_mask == U_WMR_INVALID){ throw "Inkscape fatal programming error at U_wmr_properties"; }
+    SVGOStringStream tmp_path;
+    SVGOStringStream tmp_str;
 
 /* Uncomment the following to track down text problems */
 //std::cout << "tri->dirty:"<< d->tri->dirty << " wmr_mask: " << std::hex << wmr_mask << std::dec << std::endl;
@@ -3026,14 +3035,17 @@ std::cout << "BEFORE DRAW"
 // When testing, uncomment the following to place a comment for each processed WMR record in the SVG
 //    d->outsvg += dbg_str.str().c_str();
     d->path   += tmp_path.str().c_str();
-    if(!nSize){ OK=0; std::cout << "nSize == 0, oops!!!" << std::endl; }     // There was some problem with this record, it is not safe to continue
+    if(!nSize){ // There was some problem with the processing of this record, it is not safe to continue
+        file_status = 0;
+        break;
+    } 
 
-    }  //end of while
+    }  //end of while on OK
 // When testing, uncomment the following to show the final SVG derived from the WMF
 // std::cout << d->outsvg << std::endl;
     (void) U_wmr_properties(U_WMR_INVALID);  // force the release of the lookup table memory, returned value is irrelevant
 
-    return 1;
+    return(file_status);
 }
 
 void Wmf::free_wmf_strings(WMF_STRINGS name){
@@ -3049,7 +3061,14 @@ SPDocument *
 Wmf::open( Inkscape::Extension::Input * /*mod*/, const gchar *uri )
 {
 
+    if (uri == NULL) {
+        return NULL;
+    }
+
     WMF_CALLBACK_DATA d;
+    
+    d.n_obj = 0;     //these might not be set otherwise if the input file is corrupt
+    d.wmf_obj=NULL; 
 
     // Default font, WMF spec says device can pick whatever it wants. 
     // WMF files that do not specify a  font are unlikely to look very good!
@@ -3064,12 +3083,12 @@ Wmf::open( Inkscape::Extension::Input * /*mod*/, const gchar *uri )
     d.dc[0].style.stroke_dasharray.set         = 0;
     d.dc[0].style.stroke_linecap.computed      = 2; // U_PS_ENDCAP_SQUARE;
     d.dc[0].style.stroke_linejoin.computed     = 0; // U_PS_JOIN_MITER;
-    d.dc[0].style.stroke_width.value           = 1.0; // will be reset to something reasonable once WMF draying size is known
+    d.dc[0].style.stroke_width.value           = 1.0; // will be reset to something reasonable once WMF drawing size is known
     d.dc[0].style.stroke.value.color.set( 0, 0, 0 );
+    d.dc[0].stroke_set                         = true;
 
-    if (uri == NULL) {
-        return NULL;
-    }
+    // Default brush is none - no fill. WMF files that do not specify a brush are unlikely to look very good!
+    d.dc[0].fill_set                           = false;
 
     d.dc[0].font_name = strdup("Arial"); // Default font, set only on lowest level, it copies up from there WMF spec says device can pick whatever it wants
 
@@ -3095,12 +3114,15 @@ Wmf::open( Inkscape::Extension::Input * /*mod*/, const gchar *uri )
       FT_LOAD_NO_SCALE | FT_LOAD_NO_HINTING  | FT_LOAD_NO_BITMAP,
       FT_KERNING_UNSCALED);
 
-    (void) myMetaFileProc(contents,length, &d);
+    int good = myMetaFileProc(contents,length, &d);
     free(contents);
 
 //    std::cout << "SVG Output: " << std::endl << d.outsvg << std::endl;
 
-    SPDocument *doc = SPDocument::createNewDocFromMem(d.outsvg.c_str(), strlen(d.outsvg.c_str()), TRUE);
+    SPDocument *doc = NULL;
+    if (good) {
+        doc = SPDocument::createNewDocFromMem(d.outsvg.c_str(), strlen(d.outsvg.c_str()), TRUE);
+    }
 
     free_wmf_strings(d.hatches);
     free_wmf_strings(d.images);
diff --git a/src/extension/internal/wmf-print.cpp b/src/extension/internal/wmf-print.cpp
index f750044eb..127030832 100644
--- a/src/extension/internal/wmf-print.cpp
+++ b/src/extension/internal/wmf-print.cpp
@@ -60,6 +60,7 @@
 #include "splivarot.h"             // pieces for union on shapes
 #include <2geom/svg-path-parser.h> // to get from SVG text to Geom::Path
 #include "display/canvas-bpath.h"  // for SPWindRule
+#include "display/cairo-utils.h"  // for Inkscape::Pixbuf::PF_CAIRO
 
 #include "wmf-print.h"
 
@@ -470,9 +471,8 @@ int PrintWmf::create_brush(SPStyle const *style, U_COLORREF *fcolor)
         rgba_px = (char *) pixbuf->pixels(); // Do NOT free this!!!
         colortype = U_BCBM_COLOR32;
         (void) RGBA_to_DIB(&px, &cbPx, &ct, &numCt,  rgba_px,  width, height, width * 4, colortype, 0, 1);
-        // Not sure why the next swap is needed because the preceding does it, and the code is identical
-        // to that in stretchdibits_set, which does not need this.
-        swapRBinRGBA(px, width * height);
+        // pixbuf can be either PF_CAIRO or PF_GDK, and these have R and B bytes swapped
+        if (pixbuf->pixelFormat() == Inkscape::Pixbuf::PF_CAIRO) { swapRBinRGBA(px, width * height); }
         Bmih = bitmapinfoheader_set(width, height, 1, colortype, U_BI_RGB, 0, PXPERMETER, PXPERMETER, numCt, 0);
         Bmi = bitmapinfo_set(Bmih, ct);
         rec = wcreatedibpatternbrush_srcdib_set(&brush, wht, U_DIB_RGB_COLORS, Bmi, cbPx, px);