This is a follow-up to the question asked here: Advent of Code 2023 day 1: Trebuchet (Part 1 and 2)
Changes made:
The code no longer assumes that all lines will fit into a fixed-size buffer. Although a fixed-size buffer is used, the code now checks whether a partial line was read. It continues processing and updating the
first
andlast
digits until the End of Line (EOL) is encountered. In case a partial line was read, the last 5 characters of the buffer are copied to the front (5 because the length of the longest digit name, such as eight or seven, is 5). The next read then starts 5 characters past the beginning of the buffer.Error handling has been added.
Proper sizes have been utilized (attempted).
There was a memory leak in the code because the
FILE *
was not passed tofclose()
. This issue has been fixed.Another member has been added to the
digit_map
structure that keeps track of the number of bytes that can be skipped after a digit name has been matched in part 2 of the problem. This improvement saves us multiple unnecessary iterations.For every non-numeric character, around 9 calls to
strncmp()
were being made. Whilst a prefix tree or similar structure hasn't been implemented or used, a check has been added to verify if the non-numeric character begins any digit before making all the comparisons.
Review request:
Are there any flaws or bugs in what I have implemented so far? Have I missed any cases? Do you see any off-by-one errors? Any general suggestions, style improvements, or potential undefined behavior et cetera.
Part 1:
#include <stdio.h>
#include <stdlib.h>
#include <ctype.h>
#include <string.h>
#include <errno.h>
#include <stdint.h>
#define BUFSIZE 256
struct digit_pair {
unsigned int first;
unsigned int last;
};
static struct digit_pair first_last_digits(const char *buf)
{
unsigned int first = 0;
unsigned int last = 0;
/* Note: This would return true for 0, but 0 is not part of the
* problem specification, or the input, so we don't need to check
* for it.
*/
for (size_t i = 0; buf[i]; ++i) {
if (isdigit((unsigned char) buf[i])) {
first = !first ? buf[i] - '0' : first;
last = buf[i] - '0';
}
}
return (struct digit_pair) { first, last };
}
static uintmax_t calibration_total(FILE *stream)
{
char buf[BUFSIZE];
uintmax_t total = 0;
unsigned int first = 0;
unsigned int last = 0;
while (fgets(buf, sizeof buf, stream)) {
/* If EOL was seen, parse it normally.
* Else keep the intermediate first value, and continue updating last
* until the EOL is actually seen.
*/
struct digit_pair new = first_last_digits(buf);
if (!first) {
first = new.first;
}
if (new.last) {
last = new.last;
}
const size_t len = strlen(buf);
if (len > 0 && buf[len - 1] != '\n') {
continue;
}
total += first * 10 + last;
first = last = 0;
}
return total;
}
int main(int argc, char **argv)
{
if (!argv[0]) {
fputs("Fatal - A null argv[0] was passed in.", stderr);
return EXIT_FAILURE;
}
if (argc != 2) {
fprintf(stderr, "Error - No file provided.\n"
"Usage: %s <FILE>\n", argv[0]);
return EXIT_FAILURE;
}
errno = 0;
FILE *const file = fopen(argv[1], "r");
if (!file) {
if (errno) {
perror(argv[1]);
} else {
fputs("Error - Failed to open file.fputs\n", stderr);
}
return EXIT_FAILURE;
}
const uintmax_t total = calibration_total(file);
if (ferror(file) || !feof(file)) {
fclose(file);
fputs("Error - Failed to read file.\n", stderr);
return EXIT_FAILURE;
}
printf("%ju\n", total);
fclose(file);
return EXIT_SUCCESS;
}
Part 2:
#include <stdio.h>
#include <stdlib.h>
#include <ctype.h>
#include <string.h>
#include <errno.h>
#include <stdint.h>
#define BUFSIZE 256
struct digit_pair {
unsigned int first;
unsigned int last;
};
struct digit_map {
const char *const dname;
const unsigned int val;
const unsigned int bytes_skippable; /* On a match, we can skip this amount. */
} digits[] = {
{ "one", 1, 1 },
{ "two", 2, 1 },
{ "three", 3, 3 },
{ "four", 4, 3 },
{ "five", 5, 2 },
{ "six", 6, 2 },
{ "seven", 7, 3 },
{ "eight", 8, 4 },
{ "nine", 9, 2 }
};
/* The author was not ready to part with this piece of code, so this has been commented out. */
/*static size_t get_next_pos(const char *buf) */
/*{ */
/* /1* */
/* * "one" --- Skip 1 char, as 'e' might start "eight". */
/* * "two" --- Skip 1 char, as 'o' might start "one". */
/* * "three" --- Skip 3 chars, as 'e' might start "eight". */
/* * "four" --- Skip the remaining chars. */
/* * "five" --- Skip 2 chars, as 'e' might start "eight". */
/* * "six" --- Skip the remaining chars. */
/* * "seven" --- Skip 3 chars, as 'n' might start "nine". */
/* * "eight" --- Skip the remaining chars. */
/* * "nine" --- Skips 2 chars, as 'e' might start "eight". */
/* *1/ */
/* switch (buf[0]) { */
/* case 'o': return 1; */
/* case 't': return buf[1] == 'w' ? 1 : 3; */
/* case 'f': return buf[1] == 'o' ? 3 : 2; */
/* case 's': return buf[1] == 'i' ? 2 : 3; */
/* case 'e': return 4; */
/* case 'n': return 2; */
/* } */
/* /1* Now that I think of it, none of this was necessary. */
/* * Simply add another member to the digit_map struct. */
/* *1/ */
/* return 0; */
/* } */
static struct digit_pair first_last_digits(const char *buf)
{
unsigned int first = 0;
unsigned int last = 0;
/* Note: This would return true for 0, but 0 is not part of the
* problem specification, or the input, so we don't need to check
* for it.
*/
for (size_t i = 0; buf[i]; ++i) {
if (isdigit((unsigned char) buf[i])) {
first = !first ? buf[i] - '0' : first;
last = buf[i] - '0';
} else if (strchr("otfsen", buf[i])) { /* Check if buf[i] begins any digit. */
for (size_t j = 0; j < sizeof digits / sizeof *digits; ++j) {
if (!strncmp(buf + i, digits[j].dname, strlen(digits[j].dname))) {
first = !first ? digits[j].val : first;
last = digits[j].val;
i += digits[j].bytes_skippable - 1;
break;
}
}
}
}
return (struct digit_pair) { first, last };
}
static uintmax_t calibration_total(FILE *stream)
{
char buf[BUFSIZE];
uintmax_t total = 0;
unsigned int first = 0;
unsigned int last = 0;
size_t size = sizeof buf;
char *tmp = buf;
while (fgets(tmp, size, stream)) {
/* If EOL was seen, parse it normally.
* Else keep the intermediate first value, and continue updating last
* until the EOL is actually seen.
*/
struct digit_pair new = first_last_digits(buf);
if (!first) {
first = new.first;
}
if (new.last) {
last = new.last;
}
const size_t len = strlen(buf);
if (len > 0 && buf[len - 1] != '\n') {
/* Move the last 5 characters to the start of the buffer,
* as we'd not detect it if a digit is split into two lines,
* 5, because the length of the longest digit is 5.
*/
memmove(buf, buf + len - 5, 5);
tmp = buf + 5;
/* Adjust the buffer size. The check is required because each
* subsequent read without an EOL would otherwise decrease size
* by 5.
*/
size = sizeof buf - 5 == size ? size : size - 5;
continue;
}
total += first * 10 + last;
first = last = 0;
tmp = buf;
size = sizeof buf;
}
return total;
}
int main(int argc, char **argv)
{
if (!argv[0]) {
fputs("Fatal - A null argv[0] was passed in.", stderr);
return EXIT_FAILURE;
}
if (argc != 2) {
fprintf(stderr, "Error - No file provided.\n"
"Usage: %s <FILE>\n", argv[0]);
return EXIT_FAILURE;
}
errno = 0;
FILE *const file = fopen(argv[1], "r");
if (!file) {
if (errno) {
perror(argv[1]);
} else {
fputs("Error - Failed to open file.\n", stderr);
}
return EXIT_FAILURE;
}
const uintmax_t total = calibration_total(file);
if (ferror(file) || !feof(file)) {
fclose(file);
fputs("Error - Failed to read file.\n", stderr);
return EXIT_FAILURE;
}
fclose(file);
printf("%ju\n", total);
return EXIT_SUCCESS;
}