Answer the question
In order to leave comments, you need to log in
What is the best way to manage memory?
I am writing a program to get data from Microsoft OLE2 files . I found a library with which you can pull out data. Everything is fine, everything works. But you need to improve the code, deal with leaks.
How it works?
Microsoft OLE2 files is a file system - there are directories and files (stream).
The program finds two files (/path/to/Data, /path/to/Property), reads them, extracts data, then they can be written to the database.
#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);
}
sizeof char
== 1? unsigned char* buffer
in GetReader? I tried unique_ptr, but, as I understand it, because the pointer left the scope, the data was cleared -> access violation Answer the question
In order to leave comments, you need to log in
1) sizeof(char) always and under any conditions 1 !
2) There is a magic piece SetLastError and GetLastError .
3) There are leaks in your code, as an example:
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 ?? серьёзно ?? а очистить память ?
}
1) The standard guarantees that sizeof(char) == 1
2) In c++, unfortunately, there are not many good ways to handle errors built in, so it's better to use exceptions. They are clear to everyone, you won't have to create CFB::CompoundFileReader in case of an error in GetReader and there won't be a sheet of ifs for each function call. But don't forget to clear memory and close files before throwing an exception. It is better to use RAII .
3) Smart pointers are actually not such bad memory managers, why not use them. From ReadStream, you can simply return unique_ptr, and in the case of a buffer for CFR, if their lifetime should be the same, then why not store them together.
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;
};
Didn't find what you were looking for?
Ask your questionAsk a Question
731 491 924 answers to any question