Skip to content

Commit

Permalink
Prefer C++ constructs to C-style sizeof-based macros
Browse files Browse the repository at this point in the history
  • Loading branch information
Rangi42 committed Jan 24, 2025
1 parent 84f59e1 commit b8e2c91
Show file tree
Hide file tree
Showing 7 changed files with 25 additions and 26 deletions.
11 changes: 5 additions & 6 deletions include/helpers.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -93,15 +93,14 @@ static inline int clz(unsigned int x) {
#define CAT(x, y) x##y
#define EXPAND_AND_CAT(x, y) CAT(x, y)

// Obtaining the size of an array; `arr` must be an expression, not a type!
// (Having two instances of `arr` is OK because the contents of `sizeof` are not evaluated.)
#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof *(arr))

// For lack of <ranges>, this adds some more brevity
#define RANGE(s) std::begin(s), std::end(s)

// MSVC does not inline `strlen()` or `.length()` of a constant string, so we use `sizeof`
#define QUOTEDSTRLEN(s) (sizeof(s) - 1)
// MSVC does not inline `strlen()` or `.length()` of a constant string
template<size_t N>
static constexpr size_t literal_strlen(char const (&)[N]) {
return N - 1;
}

// For ad-hoc RAII in place of a `defer` statement or cross-platform `__attribute__((cleanup))`
template<typename T>
Expand Down
26 changes: 13 additions & 13 deletions src/asm/lexer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -500,27 +500,27 @@ BufferedContent::~BufferedContent() {
}

void BufferedContent::advance() {
assume(offset < ARRAY_SIZE(buf));
assume(offset < std::size(buf));
offset++;
if (offset == ARRAY_SIZE(buf))
if (offset == std::size(buf))
offset = 0; // Wrap around if necessary
assume(size > 0);
size--;
}

void BufferedContent::refill() {
size_t target = ARRAY_SIZE(buf) - size; // Aim: making the buf full
size_t target = std::size(buf) - size; // Aim: making the buf full

// Compute the index we'll start writing to
size_t startIndex = (offset + size) % ARRAY_SIZE(buf);
size_t startIndex = (offset + size) % std::size(buf);

// If the range to fill passes over the buffer wrapping point, we need two reads
if (startIndex + target > ARRAY_SIZE(buf)) {
size_t nbExpectedChars = ARRAY_SIZE(buf) - startIndex;
if (startIndex + target > std::size(buf)) {
size_t nbExpectedChars = std::size(buf) - startIndex;
size_t nbReadChars = readMore(startIndex, nbExpectedChars);

startIndex += nbReadChars;
if (startIndex == ARRAY_SIZE(buf))
if (startIndex == std::size(buf))
startIndex = 0;

// If the read was incomplete, don't perform a second read
Expand All @@ -534,7 +534,7 @@ void BufferedContent::refill() {

size_t BufferedContent::readMore(size_t startIndex, size_t nbChars) {
// This buffer overflow made me lose WEEKS of my life. Never again.
assume(startIndex + nbChars <= ARRAY_SIZE(buf));
assume(startIndex + nbChars <= std::size(buf));
ssize_t nbReadChars = read(fd, &buf[startIndex], nbChars);

if (nbReadChars == -1)
Expand Down Expand Up @@ -720,7 +720,7 @@ int LexerState::peekChar() {
auto &cbuf = content.get<BufferedContent>();
if (cbuf.size == 0)
cbuf.refill();
assume(cbuf.offset < ARRAY_SIZE(cbuf.buf));
assume(cbuf.offset < std::size(cbuf.buf));
if (cbuf.size > 0)
return static_cast<uint8_t>(cbuf.buf[cbuf.offset]);
}
Expand Down Expand Up @@ -748,11 +748,11 @@ int LexerState::peekCharAhead() {
return static_cast<uint8_t>(view.span.ptr[view.offset + distance]);
} else {
auto &cbuf = content.get<BufferedContent>();
assume(distance < ARRAY_SIZE(cbuf.buf));
assume(distance < std::size(cbuf.buf));
if (cbuf.size <= distance)
cbuf.refill();
if (cbuf.size > distance)
return static_cast<uint8_t>(cbuf.buf[(cbuf.offset + distance) % ARRAY_SIZE(cbuf.buf)]);
return static_cast<uint8_t>(cbuf.buf[(cbuf.offset + distance) % std::size(cbuf.buf)]);
}

// If there aren't enough chars, give up
Expand Down Expand Up @@ -2339,7 +2339,7 @@ Capture lexer_CaptureRept() {
endCapture(capture);
// The final ENDR has been captured, but we don't want it!
// We know we have read exactly "ENDR", not e.g. an EQUS
capture.span.size -= QUOTEDSTRLEN("ENDR");
capture.span.size -= literal_strlen("ENDR");
return capture;
}
depth--;
Expand Down Expand Up @@ -2385,7 +2385,7 @@ Capture lexer_CaptureMacro() {
endCapture(capture);
// The ENDM has been captured, but we don't want it!
// We know we have read exactly "ENDM", not e.g. an EQUS
capture.span.size -= QUOTEDSTRLEN("ENDM");
capture.span.size -= literal_strlen("ENDM");
return capture;

default:
Expand Down
2 changes: 1 addition & 1 deletion src/asm/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ static std::string make_escape(std::string &str) {
break;
escaped.append(str, pos, nextPos - pos);
escaped.append("$$");
pos = nextPos + QUOTEDSTRLEN("$");
pos = nextPos + literal_strlen("$");
}
escaped.append(str, pos, str.length() - pos);
return escaped;
Expand Down
6 changes: 3 additions & 3 deletions src/asm/warning.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -149,16 +149,16 @@ void processWarningFlag(char const *flag) {
if (rootFlag.starts_with("error=")) {
// `-Werror=<flag>` enables the flag as an error
state = {.state = WARNING_ENABLED, .error = WARNING_ENABLED};
rootFlag.erase(0, QUOTEDSTRLEN("error="));
rootFlag.erase(0, literal_strlen("error="));
} else if (rootFlag.starts_with("no-error=")) {
// `-Wno-error=<flag>` prevents the flag from being an error,
// without affecting whether it is enabled
state = {.state = WARNING_DEFAULT, .error = WARNING_DISABLED};
rootFlag.erase(0, QUOTEDSTRLEN("no-error="));
rootFlag.erase(0, literal_strlen("no-error="));
} else if (rootFlag.starts_with("no-")) {
// `-Wno-<flag>` disables the flag
state = {.state = WARNING_DISABLED, .error = WARNING_DEFAULT};
rootFlag.erase(0, QUOTEDSTRLEN("no-"));
rootFlag.erase(0, literal_strlen("no-"));
} else {
// `-W<flag>` enables the flag
state = {.state = WARNING_ENABLED, .error = WARNING_DEFAULT};
Expand Down
2 changes: 1 addition & 1 deletion src/gfx/process.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -623,7 +623,7 @@ static std::tuple<DefaultInitVec<size_t>, std::vector<Palette>>
for (uint16_t cgbColor : list) {
ptr += snprintf(ptr, sizeof(", $XXXX"), ", $%04x", cgbColor);
}
return &buf[QUOTEDSTRLEN(", ")];
return &buf[literal_strlen(", ")];
};

// Iterate through proto-palettes, and try mapping them to the specified palettes
Expand Down
2 changes: 1 addition & 1 deletion src/link/object.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,7 @@ void obj_ReadFile(char const *fileName, unsigned int fileID) {
int matchedElems;

if (fscanf(file, RGBDS_OBJECT_VERSION_STRING "%n", &matchedElems) == 1
&& matchedElems != QUOTEDSTRLEN(RGBDS_OBJECT_VERSION_STRING))
&& matchedElems != literal_strlen(RGBDS_OBJECT_VERSION_STRING))
errx("%s: Not a RGBDS object file", fileName);

verbosePrint("Reading object file %s\n", fileName);
Expand Down
2 changes: 1 addition & 1 deletion src/version.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ extern "C" {
#endif

char const *get_package_version_string() {
if constexpr (QUOTEDSTRLEN(BUILD_VERSION_STRING) > 0) {
if constexpr (literal_strlen(BUILD_VERSION_STRING) > 0) {
return BUILD_VERSION_STRING;
}
// Fallback if version string can't be obtained from Git
Expand Down

0 comments on commit b8e2c91

Please sign in to comment.