Problem loading file to memory

Question

I am trying to load several files into memory (small file), but the problem is that when I try to load another file soon after, all the files get the same name as the last uploaded file.

I am using a struct containing name, name size, data and data size to represent the files.

// Cabeçalho
struct AssetHeader
{
    char* version;
    long version_sz;
    int files_num;
};

// Estrutura para representar os arquivos
struct AssetData
{
    char* name;
    char* data;
    long name_sz;
    long data_sz;
};

class AssetManager
{
    protected:
        FILE* m_pFile;
        AssetHeader* m_pHeader;

        std::vector<AssetData*> m_pAssets;
        unsigned int m_FileNum;

    .....
    .....

    public:
        inline std::vector<AssetData*> getFileList() {return m_pAssets;}
        inline int getFileCount() {return (int)m_FileNum;}

        AssetData* getFileByName(std::string name);

        void createAsset(const char* name="DefaultAssetName.asset");
        void loadAsset(const char* filename);

        void addFile(const char* filename);
        void saveFile(AssetData* asset, const char* filename);

        void clear();
        void writeAsset();
        void closeAsset();

    protected:
        void writeAssetHeader();
        void writeAssetBlock(AssetData* asset);

        void readAssetHeader();
        void readAssetBlock();
};


// Carrega um arquivo na memória
void AssetManager::addFile(const char* filename)
{
    AssetData* asset = new AssetData();

    FILE* file = fopen(filename, "rb");

    fseek(file, 0, SEEK_END);
    asset->data_sz = ftell(file);
    fseek(file, 0, SEEK_SET);

    asset->data = new char[asset->data_sz];

    if(fread(&asset->data[0], sizeof(char), asset->data_sz, file) == 0) {
        delete asset;
        asset = NULL;

        fclose(file);
    }

    fclose(file);

    asset->name_sz = strlen(filename);
    asset->name = (char*)filename;

    m_FileNum += 1;
    m_pAssets.push_back(asset);
    m_pHeader->files_num = m_FileNum;
}

// Utilização
int main(int argc, char* argv[])
{
    AssetManager* asset_mgr = new AssetManager();
    asset_mgr->createAsset("asset.txt");

    std::string name;

    name = "1.txt";
    asset_mgr->addFile(name.c_str());

    name = "2.txt";
    asset_mgr->addFile(name.c_str());

    asset_mgr->writeAsset();
    asset_mgr->closeAsset();

    return 0;
}

I don’t know what I’m doing wrong. I did a test using a print in each HAssetData stored but the problem is as I said, the files get the name of the last loaded file.

Answer 1

The problem is that you’re guarding the pointer received directly into its structure, which means to save the pointer to the string contained in the variable name. Therefore, when the pointed content changes to "2.txt", it is this content pointed in both instances.

Ideally you should allocate your own area for name storage and copy the content with strlen:

asset->name = new char[asset->name_sz];
strncpy(asset->name, filename, asset->name_sz);

---

Note also that in the deletion of your structures you also need to release the memory used by the internal elements. For example, in your current code you have a potential memory leak (memory Leak) in the treatment of failure of fread:

. . .
if(fread(&asset->data[0], sizeof(char), asset->data_sz, file) == 0) {
    delete asset;
    asset = NULL;

    fclose(file);
}
. . .

In case of failure you directly execute the delete in the variable asset but do not delete the memory allocated to asset->data. Since you are using C++ (at least it is in the question tag), I suggest you use classes instead of structures so you can try to ensure that the proper memory releases are made in the destructor.

Answer 2

Looking fast at your code, I realized this:

std::string name;

name = "1.txt";
asset_mgr->addFile(name.c_str());

name = "2.txt";
asset_mgr->addFile(name.c_str());

You have only 1 string named name, which takes "1.txt" and then is overwritten with "2.txt". When passing to addFile, you are using the c_str() method, which takes only the pointer to the beginning of the string.

But if you save only the pointer, this happens, the modification in one place affects another (or worse, if the string is relocated, the old pointer is invalid and can cause a crash or display invalid data).

If you only use Std::string, every assignment or parameter passage will be copied (which can be avoided with const reference: const std::string &). If you really want to use pointers, you will have to decide very carefully where to make copies, and where to keep pointers (making sure they remain valid for as long as necessary)