emberanimate can't read palette file

[Benjamin Murauer]

Hi,

I’ve not been using fractorium for a while. Now after updating to the latest version (55c043681238af22909678faf8847dfc62de87da), I’ve tried to run emberanimate on some flames that I've used in the past for testing, and i got the error message:

/usr/share/fractorium/flam3-palettes.xml:23136: parser error : Extra content at the end of the document

This seemed weird, as I certainly didn’t touch the flame palette file. The file seemed perfectly valid XML.

I was able to solve the problem by adding false to line 204 in PaletteList.cpp:

const  char* loc = __FUNCTION__;

if (ReadFile(filename.c_str(), buf, false))   // <-- libxml does not seem to like the null byte 
{
  ...

I am not a C or C++ programmer, so I don’t fully understand the implications of it, and I imagine that this is something fundamental, and someone else must have detected this issue?

Thanks so much for this absolutely awesome piece of software <3 here’s some stuff I made with it: https://www.youtube.com/watch?v=w8k2okKLpMI

Benjamin

[Matt Feemster]

Hi Ben.

We changed the line endings of many of the files from Unix to Windows in April of last year, but I’m not seeing anything in the commit log that would have affected what you are doing. Are you running Linux or Windows?

Do you see this bug only on the command line or in Fractorium as well?

I am unable to reproduce this bug on Windows, however I have committed an attempted fix. Can you please update your code and remove your edit and test again?

Glad to see you are doing some animations. We have been working on making a big 4K 60fps animation video on and off over the past few months. In doing so, I am realizing the workflow of the animator is suboptimal. So I have been adding a few fixes here and there. I’m extremely busy right now, but hopefully I’ll be able to release these improvements in a few months or so.

‌

[Benjamin Murauer]

Sorry for the delay.

I’m running on Arch Linux, and everything runs fine inside Fractorium itself even before your patch.

Unfortunately, the fix did not solve my problem. Might this be because the same check would have to be applied to resizing the buffer in line 335?

Thanks

‌

[Matt Feemster]

So you are saying it runs fine when running Fractorium, but fails when running emberanimate on the command line?

[Matt Feemster]

Good catch on line 335. I’ve committed another fix. Please test.

[Matt Feemster]

Following back up on this. Did any of my last commits fix the problem?

[Benjamin Murauer]

Hi,

unfortunately, it does not solve the issue. I guess libxml does not even want a single null byte at the end of the palette file.

‌

[Matt Feemster]

So strange, I am not sure what else to try at this point.

[Benjamin Murauer]

I’m sorry I can't be of more help. I’ve boiled it down to the following script, which fails on line 27 on my Linux system:

#include <fstream>
#include <string>
#include <iostream>
#include <libxml/xmlreader.h>

using namespace std;

int main(int argc, char** argv)
{
    string filename = "test.xml";
    string buf;
    bool nullTerminate = true;
    try
    {
      ifstream ifs;
      ifs.exceptions(ifstream::failbit);
      ifs.open(filename, ios::binary | ios::ate);

      if (const auto pos = ifs.tellg())//Ensure it exists and wasn't empty.
      {
        buf.resize(pos);// +streampos(nullTerminate ? 1 : 0));
        ifs.seekg(0, ios::beg);
        ifs.read(&buf[0], pos);

        if (nullTerminate && (buf[buf.size() - 1] != 0))//Optionally NULL terminate if they want to treat it as a string, and it's not terminated arleady.
          buf.push_back(0);
          const auto doc = xmlReadMemory(static_cast<const char*>(buf.data()), static_cast<int>(buf.size()), filename.c_str(), nullptr, XML_PARSE_NONET);
      }
    }
    catch (const std::exception& e)
    {
      cout << "Error: Reading file " << filename << " failed: " << e.what() << "\n";
    }
    catch (...)
    {
      cout << "Error: Reading file " << filename << " failed.\n";
    }    
    return 0;
}

Either of the following changes works for me:

• setting nullTerminate to false in the calling PalletteList.cpp on line 204
• decreasing the content size by 1 in line 27, so it becomes static_cast<int>(buf.size()-1)
• reading the file using xmlCtxtReadFile from libxml (see this example) instead of manually reading its contents

‌

Let me know if I can help any further.

[Matt Feemster]

I’ve committed a fix and now realize I’ve been looking at this wrong the entire time.

The whole “null terminate” functionality was added a long time ago because when I originally wrote it, I had it returning a vector<char>, which needed to be manually null terminated. Sometime later, I changed it to std::string, which of course does the null termination automatically. I forgot to remove the manual null termination code after that.

‌

I’ve since removed it and it always will return just a regular string, which is automatically null terminated.

‌

Thanks for catching this, I’d have never noticed otherwise.

‌

Confirm it works on linux for you and I’ll close this ticket.

[Benjamin Murauer]

The code runs fine with this fix, thanks : )

[Matt Feemster]

Fixed.

[Matt Feemster]

Closed.