As many suggestions were given for the code I posted here, I decided to show you the new version for review.
I Removed the
OpenCatalog
class,Disassembler
now has the responsibility for handling the catalog ofOpCode
that is now stored in anstd::map
.OpCode
is now astruct
andAssemblyLine
handles the generation of the assembler code.FileReaderHelper
is nowfile_parse_helpers
, a namespace containing helper functions.- I used
string_view
everywhere I though I could. - I fixed all the things like initialization order, includes, spelling, consts, etc
- Reduced the amount of streams I was using.
file_parse_helpers.h
#ifndef FILE_PARSER_HELPER_H
#define FILE_PARSER_HELPER_H
#include <vector>
#include <string>
namespace file_parse_helpers
{
std::vector<std::string_view> splitString(std::string_view toSplit, std::string_view delimiter);
int hexStringToInt(std::string_view toSplit);
std::vector<unsigned char> loadBinaryFile(std::string filepath);
}
#endif // FILE_PARSER_HELPER_H
file_parse_helpers.cpp
#include "FileReaderHelper.h"
#include <fstream>
#include <algorithm>
namespace file_parse_helpers
{
std::vector<std::string_view> splitString(std::string_view toSplit, std::string_view delimiter)
{
std::vector<std::string_view> result;
size_t lastPos = 0;
while (lastPos < toSplit.size()) {
const auto position = toSplit.find_first_of(delimiter, lastPos);
if (lastPos != position) {
result.emplace_back(toSplit.substr(lastPos, position - lastPos));
}
if (position == std::string_view::npos)
break;
lastPos = position + 1;
}
return result;
}
int hexStringToInt(std::string_view toSplit)
{
const auto pouet = std::strtol(toSplit.data(), 0, 16);
return pouet;
}
std::vector<unsigned char> file_parse_helpers::loadBinaryFile(std::string filepath)
{
std::ifstream inputStream(filepath, std::ios::binary);
std::vector<unsigned char> result(std::istreambuf_iterator<char>{inputStream}, {});
return result;
}
}
OpCode.h
#ifndef OPCODE_H
#define OPCODE_H
#include <string>
struct OpCode {
uint8_t code;
int byteCount;
std::string resultTemplate;
};
#endif // OPCODE_H
AssemblyLine.h
#ifndef ASSEMBLYLINE_H
#define ASSEMBLYLINE_H
#include <string>
#include "OpCode.h"
class AssemblyLine
{
public:
AssemblyLine(const OpCode& opCode);
AssemblyLine(const OpCode& opCode, const uint8_t param1);
AssemblyLine(const OpCode& opCode, const uint8_t param1, const uint8_t param2);
uint8_t getParam1() const;
uint8_t getParam2() const;
uint8_t getCode() const;
uint8_t getByteCount() const;
std::string generateAssembler(const std::string& templateString) const;
protected:
const uint8_t byteCount;
const uint8_t code;
const uint8_t param1;
const uint8_t param2;
};
#endif
AsemblyLine.cpp
#include "AssemblyLine.h"
#include <stdexcept>
AssemblyLine::AssemblyLine(const OpCode& opCode) :
byteCount(opCode.byteCount),
code(opCode.code),
param1(0),
param2(0)
{
}
AssemblyLine::AssemblyLine(const OpCode& opCode, const uint8_t param1) :
byteCount(opCode.byteCount),
code(opCode.code),
param1(param1),
param2(0)
{
}
AssemblyLine::AssemblyLine(const OpCode& opCode, const uint8_t param1, const uint8_t param2) :
byteCount(opCode.byteCount),
code(opCode.code),
param1(param1),
param2(param2)
{
}
uint8_t AssemblyLine::getParam1() const
{
return param1;
}
uint8_t AssemblyLine::getParam2() const
{
return param2;
}
uint8_t AssemblyLine::getCode() const
{
return code;
}
uint8_t AssemblyLine::getByteCount() const
{
return byteCount;
}
std::string AssemblyLine::generateAssembler(const std::string& templateString) const
{
const int maxStringLength{ 32 };
if (templateString.length() > maxStringLength)
{
throw std::invalid_argument("This template string is too long: " + templateString);
}
char temp[maxStringLength];
switch (byteCount) {
case 1:
return templateString;
case 2:
std::snprintf(temp, maxStringLength, templateString.c_str(), param1);
break;
case 3:
std::snprintf(temp, maxStringLength, templateString.c_str(), param1, param2);
break;
}
return temp;
}
Disassembler.h
#ifndef DISASSEMBLER_H
#define DISASSEMBLER_H
#include <map>
#include <vector>
#include "AssemblyLine.h"
#include "OpCode.h"
class Disassembler
{
public:
Disassembler(const std::string& filename);
std::vector<AssemblyLine> disassemble(const std::vector<unsigned char>& data);
void printAssembly(const std::vector<unsigned char>& data);
AssemblyLine disasembleOneCode(const std::vector<unsigned char>& data, size_t codeIndex);
void loadCatalog(const std::string& filename);
private:
std::map<uint8_t, OpCode> opCodes;
};
#endif
Disassembler.cpp
#include "Disassembler.h"
#include "File/FileReaderHelper.h"
#include <iostream>
#include <fstream>
Disassembler::Disassembler(const std::string& filename)
{
loadCatalog(filename);
}
AssemblyLine Disassembler::disasembleOneCode(const std::vector<unsigned char>& data, size_t codeIndex)
{
uint8_t codeValue = data[codeIndex];
const auto& opCode = opCodes[codeValue];
const auto byteCount = opCode.byteCount;
switch (byteCount) {
default:
case 1:
return AssemblyLine(opCode);
break;
case 2:
return AssemblyLine(opCode, data[codeIndex + 1]);
break;
case 3:
return AssemblyLine(opCode, data[codeIndex + 2], data[codeIndex + 1]);
break;
}
}
std::vector<AssemblyLine> Disassembler::disassemble(const std::vector<unsigned char>& data)
{
size_t currentId = 0;
std::vector<AssemblyLine> disasembledCode;
for (; currentId < data.size() - 1;)
{
const auto asmLine = disasembleOneCode(data, currentId);
currentId += asmLine.getByteCount();
disasembledCode.emplace_back(asmLine);
}
return disasembledCode;
}
void Disassembler::loadCatalog(const std::string& filename)
{
opCodes.clear();
std::ifstream input(filename);
for (std::string line; std::getline(input, line);)
{
const auto tokens = file_parse_helpers::splitString(line, "\t");
if (tokens.size() < 3) {
throw "Malformed catalog line: " + line;
}
uint8_t code = file_parse_helpers::hexStringToInt(std::string(tokens[0]));
opCodes.emplace(code, OpCode{ code,
std::stoi(std::string(tokens[2]), nullptr, 10),
std::move(std::string(tokens[1])) });
}
}
void Disassembler::printAssembly(const std::vector<unsigned char>& data)
{
size_t currentId = 0;
for (; currentId < data.size() - 1;)
{
const auto asmLine = disasembleOneCode(data, currentId);
currentId += asmLine.getByteCount();
std::cout << asmLine.generateAssembler(opCodes[asmLine.getCode()].resultTemplate) << std::endl;
}
}
Edit: After posting I realized that I made a mistake. I throw an exception about the size of the template string provided in the catalog, but I should do it when parsing the catalog, not when I use the string. Now, how do I replace maxStringLength
so that I can use it when loading the catalog and as the size of the temp
array in generateAssembler
? Are C macros the only solution here?