Каким образом лучше управлять памятью?

Question

[specific-blueberry]

Пишу программу для получения данных из Microsoft OLE2 files. Нашёл библиотеку, с помощью которой можно вытаскивать данные. Всё хорошо, всё работает. Но нужно улучшать код, бороться с утечками.
Как это работает?
Microsoft OLE2 files представляет собой файловую систему - там есть директории и файлы (stream).
Программа находит два файла (/path/to/Data, /path/to/Property), читает их, извлекает данные, далее их можно записать в БД.

#include <iostream>
#include "com.h"
#include "utf.h"
#include <windows.h>
#include <cstdint>
#include <algorithm>
#include <sstream>
#include <iomanip>
#include <vector> 
#pragma execution_character_set( "utf-8" )

const CFB::COMPOUND_FILE_ENTRY* FindStream(const CFB::CompoundFileReader& reader, const char* streamName) {
    const CFB::COMPOUND_FILE_ENTRY* ret = nullptr;

    reader.EnumFiles(reader.GetRootEntry(), -1,
        [&](const CFB::COMPOUND_FILE_ENTRY* entry, const CFB::utf16string& u16dir, int level)->void {

            if (reader.IsStream(entry)) {
                std::string name = UTF16ToUTF8(entry->name);
                
                if (u16dir.length() > 0) {
                
                    std::string dirName = UTF16ToUTF8(u16dir.c_str(), u16dir.length());
                    std::replace(dirName.begin(), dirName.end(), '\n', '\\');
                    dirName.erase(std::remove(dirName.begin(), dirName.end(), (char) 0x00), dirName.end());
                    std::string fullName = dirName + '\\' + name;
                    if (strcmp(fullName.c_str(), streamName) == 0) {
                        ret = entry;
                    }
                }
                else {
                    if (strcmp(streamName, name.c_str()) == 0)
                    {
                        ret = entry;
                    }
                }
            }
        }
    );
    return ret;
}

char* ReadStream(const CFB::CompoundFileReader& reader, const CFB::COMPOUND_FILE_ENTRY* entry) {
    char* buf = new char[entry->size];
    reader.ReadFile(entry, 0, buf, entry->size);
    return buf;
}

void ExtractFromDataStream(const CFB::CompoundFileReader& reader, const CFB::COMPOUND_FILE_ENTRY* entry) {
    char* buf = ReadStream(reader, entry);
    ...
    delete[] buf;
}

// returns some struct, not void.
void ExtractFromPropertyStream(const CFB::CompoundFileReader& reader, const CFB::COMPOUND_FILE_ENTRY* entry) {
    char* buf = ReadStream(reader, entry);
    ...
    delete[] buf;
}

const CFB::CompoundFileReader GetReader(std::string path) {
    FILE* fp;
    fopen_s(&fp, path.c_str(), "rb");
    if (fp == NULL) {
        std::cerr << "read file error" << std::endl;
    }

    fseek(fp, 0, SEEK_END);
    size_t len = ftell(fp);
    unsigned char* buffer = new unsigned char[len];
    fseek(fp, 0, SEEK_SET);
    len = fread(buffer, 1, len, fp);
    fclose(fp);
    CFB::CompoundFileReader reader(buffer, len);
    return reader;
}

int main()
{
    SetConsoleCP(1251);
    SetConsoleOutputCP(1251);
    std::string path = "path\\to\\file.dat";
    auto reader = GetReader(path);
    auto dataStream = FindStream(reader, "path\\to\\Data");
    if (dataStream == nullptr) {
        std::cerr << "dataStream is null" << std::endl;
        return 1;
    }
    auto propertyStream = FindStream(reader, "path\\to\\Property");
    std::cout << propertyStream->size << std::endl;
    if (propertyStream == nullptr) {
        std::cerr << "propertyStream is null" << std::endl;
        return 1;
    }
    ExtractFromPropertyStream(reader, propertyStream);
}

Я знаю, этот код ужасен, но нужно найти ошибки и их исправить.
1) Как гарантировать, что sizeof char == 1?
2) Как в этом случае лучше обрабатывать ошибки? Например, в функции GetReader не обрабатывается, что с fp может быть что-то не так. Как вариант, бросать исключение, обрабатывать в main.
3) Как быть с утечкой памяти, когда создаётся unsigned char* buffer в GetReader? Я пробовал unique_ptr, но, как я понял, из-за того, что указатель выходил из скоупа, данные очищались -> access violation
4) Как лучше всего обрабатывать данные?
5) Как в общем улучшить код?

Answer 1

[Ternick]

1) sizeof(char) всегда и при любых условиях 1 !
2) Есть волшебная штука SetLastError и GetLastError.
3)Утечки у тебя в коде присутвуют, как пример:

char* ReadStream(const CFB::CompoundFileReader& reader, const CFB::COMPOUND_FILE_ENTRY* entry) {
    char* buf = new char[entry->size]; // Круто, выделил память и всё
    reader.ReadFile(entry, 0, buf, entry->size); // использовал память
    return buf; // return ?? серьёзно ?? а очистить память ?
}

Прочитай про сборщики мусора вроде cdecl и других.
Создавай глобальные переменные и туда выделяй память, а в конце очищай, на мой взгляд лучше использовать malloc и free.
Дальше ничем не помогу.

Comments

[specific-blueberry]

Память очищается после использования данных, но я понимаю, что можно забыть очистить память -> утечка памяти. В любом случае мне нужно как-то использовать эту память, а не просто очистить её. А два раза писать

char* buf = new char[entry->size]; // Круто, выделил память и всё
    reader.ReadFile(entry, 0, buf, entry->size); // использовал память. (примечание: я здесь просто копирую данные в массив buf, а не использую её. использую её в ExtractFromDataStream, ExtractFromPropertyStream)

тоже неправильно, потому что повторение кода. В общем, я не знаю, как лучше.

[Ternick]

specific-blueberry, Ты меня не понял ! Нужно переменную объявить глобально и использовать глобальную переменную, а затем в конце освободить память. Есть вариант собирать адреса переменных в которые ты выделяешь память, а потом проходится по каждому адресу и освобождать память.

Answer 2

[xorknown]

1) Стандарт гарантирует, что sizeof(char) == 1
2) В с++, к сожалению, не так много встроенных хороших способов обработки ошибок, поэтому лучше использовать исключения. Они понятны всем, не придется создавать CFB::CompoundFileReader в случае ошибки в GetReader и не будет простыни из if-ов на каждый вызов функции. Но не забывай очищать память и закрывать файлы перед бросанием исключения. Лучше пользоваться RAII.
3) Smart pointer-ы на самом деле не такие уж и плохие менеджеры памяти, почему бы не использовать их. Из ReadStream можно просто возвращать unique_ptr, а в случае с буффером для CFR, если их время жизни должно совпадать, то почему бы не хранить их вместе.

class ReaderWatcher {
public:
    ReaderWatcher(std::shared_ptr<unsigned char[]> b, size_t len)
     : buffer(std::move(b)), reader(b.get(), len) {}

    Reader* operator ->() {
        return &reader;
    }

    Reader get() {
        return reader;
    }

private:
    std::shared_ptr<unsigned char[]> buffer {nullptr};
    Reader reader;
};