Return to Revisions

This is a supplement to the excellent answer by Edward.

Care with namespaces

Currently, the header has

using std::uint16_t;
using std::uint8_t;

I recommend not bringing these into the global namespace in a header - that affects every translation unit that uses the header, which can be a nuisance in larger programs (particularly when not all written by the same author). Instead, if you really feel that std:: is too much to type and to read, bring them into a smaller scope (e.g. within a function, or at worst into global scope in individual, non-header, files).

There are a few uses of unqualified names from the std namespace - these should be portably written std::size_t, std::memcpy, etc. You've probably only compiled on systems that use their freedom to put copies of Standard Library identifiers into the global namespace, but that's not required, and you can't depend on it.

Includes

I recommend re-ordering these includes:

#include <cerrno>
#include <limits>
#include <cstdlib>
#include <cstring>
#include <fstream>
#include <iostream>
#include <exception>
#include "z80emu.hpp"

If we put our own includes before the standard library headers, we stand a better chance of identifying accidental dependencies:

#include "z80emu.hpp"
#include <cerrno>
#include <cstdlib>
#include <cstring>
#include <exception>
#include <fstream>
#include <iostream>
#include <limits>

I'm pretty sure we don't use <limits>, and really ought to have <cstdint> instead (for UINT16_MAX and the like).

Error reporting

Most error messages are correctly sent to std::cerr. But when we call usage() to indicate invocation errors, that's sent to std::cout. We should pass the stream to this function, too, so we can make it print to the error stream when it's shown as an error (rather than specifically requested, when we add support for --help argument).

static void usage(std::ostream& os, const char *progname)
{
 os << " Usage: " << progname << " z80-prog\n";
}

I also recommend static linkage here, as this function shouldn't need to be accessible from other translation units.