-3

I'm trying to open a file as output so that I can write converted model data to my file. The problem currently is, even though the path does exist, and the file is created, it isn't actually open. I've tried removing the !is_open() check and of course it segfaults since it isn't open. I've added ios_base modes to specify how to open the file, and tried switching to a blank ofstream constructor and implicitly calling .open() to no result.

Again, the path is absolutely correct, a file is in fact created at the right path, it just isn't opening for whatever reason. Permissions are fine, I even tried running as root to no success.

void AssetConverter::ConvertModel(const std::string importPath, std::string exportPath, ModelImportSettings* settings) {
    // assimp has a comically massive footprint which is why i create it implicitly
    Assimp::Importer* importer = new Assimp::Importer();
    const aiScene* scene = importer->ReadFile(importPath, 
        aiProcess_Triangulate 
        | aiProcess_OptimizeMeshes
        | aiProcess_OptimizeGraph
    );
    delete importer;
    if(scene == nullptr) {
        Debug::LogError("Couldn't import model from path: " + importPath);
        return;
    }

    bool skinned;
    switch (settings->isSkinnedMesh) {
        case TRI_TRUE:
            skinned = true;
            break;
        case TRI_FALSE:
            skinned = false;
            break;
        default: // Auto
            skinned = scene->HasSkeletons();
            break;
    }

    std::filesystem::path iP(importPath);
    exportPath = std::filesystem::absolute(exportPath);
    ModelBase m;
    if(skinned) {
        m = CreateSkinnedModel(scene);
        exportPath += "/" + iP.stem().string() + ".nems";
    } else {
        m = CreateModel(scene);
        exportPath += "/" + iP.stem().string() + ".nem";
    }

    if(std::filesystem::exists(exportPath)) {
        std::filesystem::remove(exportPath);
    }

    // Actual problem starts here
    std::ofstream outFile(exportPath, std::ios_base::out | std::ios_base::binary | std::ios_base::trunc);

    if(outFile.bad()) {
        Debug::LogError("Couldn't open, bad stream: " + exportPath);
        return;
    }
    if(outFile.fail()) {
        Debug::LogError("Couldn't open, failed: " + exportPath);
        return;
    }
    if(!outFile.is_open()) {
        // enters here every single time
        bool exists = std::filesystem::exists(exportPath);
        // i had comments showing the rage here, that i have gracefully removed
        Debug::LogError("Couldn't open path for writing: " + exportPath);
        if(exists) {
            Debug::LogError("BUT THE PATH DOES EXIST WHWHWHWHHATATTTATT");
        }
        return;
    }
    const std::vector<uint8_t> data = m.getData();
    for(unsigned char v : data) {
        std::string str = std::to_string(v);
        outFile.write(str.c_str(), str.length());
    }
    outFile.flush();
    outFile.close();
}
Improve this question
10
  • 7
    Do you really need all of this code to duplicate the error? A simple 3 or 4 line main program, with the attempt to open a file with a hardcoded name is all that would have been necessary to duplicate the issue. Commented Nov 25 at 22:55
  • 5
    From the documentation of Assimp::Importer::ReadFile: /.../ the contents of the file are returned as a pointer to an aiScene object. The returned data is intended to be read-only, the importer object keeps ownership of the data and will destroy it upon destruction. (Emphasis mine.) This means that after you delete importer, the data pointed to by scene is gone! No surprise you get a segfault. Commented Nov 25 at 23:10
  • 2
    Side note: you don’t need outFile.flush(); outFile.close();. The destructor will do that. Commented Nov 25 at 23:15
  • 2
    If the text of your question is adequate, your minimal reproducible example should start from // Actual problem starts here with the path hardcoded (instead of exportPath) and error messages sent to std::cout or std::cerr instead of whatever Debug::LogError is (minimize dependencies on other code). It can end after the three checks (bad, fail, open) since you already have your symptom at that point. Furthermore, this should become your main function instead of a member function of some unknown class, so that you can copy-compile-run the code to reproduce the result. Commented Nov 25 at 23:22
  • 2
    "the path is absolutely correct" -- People who claim "absolutely correct" without providing evidence have a tendency to be wrong. Please provide an example path for which this code fails. (You could even create a test directory for this example to keep the example path short.) Commented Nov 25 at 23:27

1 Answer 1

Reset to default
-5

Just let Assimp::Importer live on the stack and don’t delete it manually:

void AssetConverter::ConvertModel(const std::string importPath,
                                  std::string exportPath,
                                  ModelImportSettings* settings) 
{
    // Create Importer on the stack
    Assimp::Importer importer;
    const aiScene* scene = importer.ReadFile(importPath,
        aiProcess_Triangulate 
        | aiProcess_OptimizeMeshes
        | aiProcess_OptimizeGraph
    );

    if (scene == nullptr) {
        Debug::LogError("Couldn't import model from path: " + importPath);
        return;
    }

    // SAFE: importer is still alive, so scene is valid
    bool skinned;
    switch (settings->isSkinnedMesh) {
        case TRI_TRUE:  skinned = true;  break;
        case TRI_FALSE: skinned = false; break;
        default:        skinned = scene->HasSkeletons(); break;
    }

    std::filesystem::path iP(importPath);
    exportPath = std::filesystem::absolute(exportPath).string();

    ModelBase m;
    if (skinned) {
        m = CreateSkinnedModel(scene);
        exportPath += "/" + iP.stem().string() + ".nems";
    } else {
        m = CreateModel(scene);
        exportPath += "/" + iP.stem().string() + ".nem";
    }

    if (std::filesystem::exists(exportPath)) {
        std::filesystem::remove(exportPath);
    }

    std::ofstream outFile(exportPath,
        std::ios_base::out | std::ios_base::binary | std::ios_base::trunc);

    if (!outFile) {
        Debug::LogError("Couldn't open file for writing: " + exportPath);
        return;
    }

    const std::vector<uint8_t> data = m.getData();
    for (unsigned char v : data) {
        std::string str = std::to_string(v);
        outFile.write(str.c_str(), str.length());
    }
}
Improve this answer
New contributor
YISUSVII Crt is a new contributor to this site. Take care in asking for clarification, commenting, and answering. Check out our Code of Conduct.
Sign up to request clarification or add additional context in comments.

4 Comments

This is probably correct, but I strongly recommend explaining how this helps. Even if the problem turns out to be something else, the lifetime of importer needs fixing.
Typically a good answer explains what went wrong with the asker's code, offers a solution, and explains how the the proposed solution solves the problem. Sometimes you can skip one of these three parts, but the problem has to be very trivial to be able to skip two.
Thanks, I did initially have it on the stack, but I really didn't like the memory footprint of the importer so I chose to allocate it on the heap so I could delete it as soon as possible, even if it isn't as safe. Definitely was a mistake though where I deleted it, should have been after I use scene to create a model instance. I believe at the time of writing that bit of code, I thought ReadFile() returned a copy. With those fixes, I still do get the issues with the ofstream not being open, which was my initial main issue I wanted to fix, however I'm definitely fixing my usages of Assimp.
so I could delete it as soon as possible -- You should have used RAII, where you create a class that holds the Assimp pointer, and in the destructor of the class, the Assimp pointer gets destroyed. You then create an instance of this class within your ConvertModel function, and assign the pointer to this instance. Then there is no need to guess when or where to delete the pointer, since the destruction of that instance will be automatic, and will always delete the pointer when the function returns.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.