Skip to main content
fixed name of main routine
Source Link
Edward
  • 66.6k
  • 4
  • 118
  • 282
int diassemblediassembleOne(std::vector<char>::iterator& it, std::vector<char>::iterator& end) {
    //  figure out number of bytes for this opcode
    if (std::distance(it, end) > opbytes) {
        it = end;
        // throw or return 0
    }
    // disassemble the instruction thoroughly
    std::advance(it, opbytes);
    return opbytes;
}
class Opcode {
    char code;
    short int bytes;
    charstd::string_view *name;name;
    bool operator==(char opcode) const { return code == opcode; }
    int decode(std::vector<char>::const_iterator& it, std::ostream& out=std::cout) const {
        out << name; 
        ++it;
        for (int i{bytes-1}; i; --i) {
            out << static_cast<unsigned>(*it++);
        }
        out << '\n';
        return bytes;
    }
};

constexpr std::array<Opcode,2> instructions {
    { 0x10, 2, "STOP $" },
    { 0x76, 2, "HALT " },
};
int disassembledisassembleOne(std::vector<char>::const_iterator& it, std::vector<char>::const_iterator& end){
    auto op{std::find(instructions.begin(), instructions.end(), *it)};
    if (op == instructions.end()) {
        std::cout << "Instruction not found\n";
        it = end;
        return 0; // instruction not found or off the end of the passed array
    }
    if (std::distance(it, end) < op->bytes) {
        std::cout << "Not enough bytes left to decode " << op->name << '\n';
        it = end;
        return 0; // instruction not found or off the end of the passed array
    }
    return op->decode(it);
}
int main(){
    const std::vector<char> char_vect = {0x76, 0x10, 0x20, 0x10}; // Fill vector of char
    auto end{char_vect.cend()};

    for (auto it{char_vect.cbegin()}; it != end; disassembledisassembleOne(it, end)) {
    }
}
int diassemble(std::vector<char>::iterator& it, std::vector<char>::iterator& end) {
    //  figure out number of bytes for this opcode
    if (std::distance(it, end) > opbytes) {
        it = end;
        // throw or return 0
    }
    // disassemble the instruction thoroughly
    std::advance(it, opbytes);
    return opbytes;
}
class Opcode {
    char code;
    short int bytes;
    char *name;
    bool operator==(char opcode) const { return code == opcode; }
    int decode(std::vector<char>::const_iterator& it, std::ostream& out=std::cout) const {
        out << name; 
        ++it;
        for (int i{bytes-1}; i; --i) {
            out << static_cast<unsigned>(*it++);
        }
        out << '\n';
        return bytes;
    }
};

constexpr std::array<Opcode,2> instructions {
    { 0x10, 2, "STOP $" },
    { 0x76, 2, "HALT " },
};
int disassemble(std::vector<char>::const_iterator& it, std::vector<char>::const_iterator& end){
    auto op{std::find(instructions.begin(), instructions.end(), *it)};
    if (op == instructions.end()) {
        std::cout << "Instruction not found\n";
        it = end;
        return 0; // instruction not found or off the end of the passed array
    }
    if (std::distance(it, end) < op->bytes) {
        std::cout << "Not enough bytes left to decode " << op->name << '\n';
        it = end;
        return 0; // instruction not found or off the end of the passed array
    }
    return op->decode(it);
}
int main(){
    const std::vector<char> char_vect = {0x76, 0x10, 0x20, 0x10}; // Fill vector of char
    auto end{char_vect.cend()};

    for (auto it{char_vect.cbegin()}; it != end; disassemble(it, end)) {
    }
}
int diassembleOne(std::vector<char>::iterator& it, std::vector<char>::iterator& end) {
    //  figure out number of bytes for this opcode
    if (std::distance(it, end) > opbytes) {
        it = end;
        // throw or return 0
    }
    // disassemble the instruction thoroughly
    std::advance(it, opbytes);
    return opbytes;
}
class Opcode {
    char code;
    short int bytes;
    std::string_view name;
    bool operator==(char opcode) const { return code == opcode; }
    int decode(std::vector<char>::const_iterator& it, std::ostream& out=std::cout) const {
        out << name; 
        ++it;
        for (int i{bytes-1}; i; --i) {
            out << static_cast<unsigned>(*it++);
        }
        out << '\n';
        return bytes;
    }
};

constexpr std::array<Opcode,2> instructions {
    { 0x10, 2, "STOP $" },
    { 0x76, 2, "HALT " },
};
int disassembleOne(std::vector<char>::const_iterator& it, std::vector<char>::const_iterator& end){
    auto op{std::find(instructions.begin(), instructions.end(), *it)};
    if (op == instructions.end()) {
        std::cout << "Instruction not found\n";
        it = end;
        return 0; // instruction not found or off the end of the passed array
    }
    if (std::distance(it, end) < op->bytes) {
        std::cout << "Not enough bytes left to decode " << op->name << '\n';
        it = end;
        return 0; // instruction not found or off the end of the passed array
    }
    return op->decode(it);
}
int main(){
    const std::vector<char> char_vect = {0x76, 0x10, 0x20, 0x10}; // Fill vector of char
    auto end{char_vect.cend()};

    for (auto it{char_vect.cbegin()}; it != end; disassembleOne(it, end)) {
    }
}
Source Link
Edward
  • 66.6k
  • 4
  • 118
  • 282

I have some ideas about how you might be able to improve your program.

Avoid problems

Rather than trying to deal with the problem for every instruction, one approach is avoiding it entirely. One way to do that is to simply append a number of bytes to the end of the vector. If the maximum bytes for an instruction is \$n\$, then append \$n-1\$ bytes to the end and stop when you've advanced into the padded area.

Check before advancing

One could also pass the number of remaining bytes to the disassemble function. However, the mechanism I'd suggest would be to pass a range, e.g.:

int diassemble(std::vector<char>::iterator& it, std::vector<char>::iterator& end) {
    //  figure out number of bytes for this opcode
    if (std::distance(it, end) > opbytes) {
        it = end;
        // throw or return 0
    }
    // disassemble the instruction thoroughly
    std::advance(it, opbytes);
    return opbytes;
}

Use const iterators

If all the code is doing is disassembling, then it shouldn't alter the underlying vector. For that reason, I'd recommend passing a std::vector<char>::const_iterator &.

Use classes

I'd suggest using an Opcode class like this:

class Opcode {
    char code;
    short int bytes;
    char *name;
    bool operator==(char opcode) const { return code == opcode; }
    int decode(std::vector<char>::const_iterator& it, std::ostream& out=std::cout) const {
        out << name; 
        ++it;
        for (int i{bytes-1}; i; --i) {
            out << static_cast<unsigned>(*it++);
        }
        out << '\n';
        return bytes;
    }
};

constexpr std::array<Opcode,2> instructions {
    { 0x10, 2, "STOP $" },
    { 0x76, 2, "HALT " },
};

Pass a pair of iterators to the dissemble function

As mentioned before, you can pass a pair of iterators to the disassemble function. Using that plus the class above:

int disassemble(std::vector<char>::const_iterator& it, std::vector<char>::const_iterator& end){
    auto op{std::find(instructions.begin(), instructions.end(), *it)};
    if (op == instructions.end()) {
        std::cout << "Instruction not found\n";
        it = end;
        return 0; // instruction not found or off the end of the passed array
    }
    if (std::distance(it, end) < op->bytes) {
        std::cout << "Not enough bytes left to decode " << op->name << '\n';
        it = end;
        return 0; // instruction not found or off the end of the passed array
    }
    return op->decode(it);
}

Now main becomes very simple:

int main(){
    const std::vector<char> char_vect = {0x76, 0x10, 0x20, 0x10}; // Fill vector of char
    auto end{char_vect.cend()};

    for (auto it{char_vect.cbegin()}; it != end; disassemble(it, end)) {
    }
}

Another way to do this would be to put more of the processing in the Opcode itself -- it would make sense that each opcode would know how to decode itself.

Be clear about caller expectations

This code, as with the original code, expects that the it being passed is a valid iterator that is not the end. It is good to document that in comments in the code.