A dice roller in C

Question

I am learning C and, for learning purposes, I implemented a dice roller.

It uses arc4random(3) for random number generation, as I couldn't find something better.

It works both interactively, if no argument is given, reading dice string from stdin, one per line (I used getline(3) for this); and reading dice string from arguments.
A dice string is a dice specification similar to those used in most role playing games, for example, rolldice 3d6 rolls 3 6-sized dice and sum them up, and rolldice 4d8+2s1 rolls 4 8-sized dice, discards one roll, sum the rolls, and add 2 to the result. More information on this on the manual.

I wrote a manual page for it, the manual page is based on the manual of Stevie Strickland's rolldice, but the code I wrote from scratch.

Here's the manual:

rolldice(6) Games Manual rolldice(6) NAME rolldice - rolls virtual dice SYNOPSIS rolldice [-s] [dice string...] DESCRIPTION rolldice rolls virtual dice. The dice strings passed on the command line contain information on the dice to roll in a format comparable to the format used in most role playing games. If no dice strings are provided as command line arguments, rolldice uses stdin as input and runs interactivelly. The options are as follows: -s Print out the result of each individual die separately, as well as the operations and totals. DICE STRING FORMAT The dice string uses the exact format outlined below. Optional parts are between square brackets. A # must be replaced by a num‐ ber. [#x][#]d[#|%][*#][+#|-#][s#] [#x] How many times to roll. If ommited, defaults to 1 roll. [#]d[#|%] Main part of the dice string. The first number is the number of dice to roll in each roll, if ommited, roll just one die. The second number is the number of sides the dice have, if ommited, roll 6-sided die. The second number can be replaced by a percent sign, implying a 100-sided die. The numbers rolled on each die are then added up and given as the result. [*#] How many times to multiply the result of each roll. [+#|-#] Number to be added or subtracted, depending on the sign, from each roll. This step is handled after the multiplication. [s#] How many lowest dice rolls to drop. This step is handled be‐ fore the multiplication. EXIT STATUS 0 Success. >0 Error occurred. EXAMPLES Roll three six-sided dice and sum the results: rolldice 3d Roll four eight-sided dice and sum the results, them multiply the result by 2 and add 2 to it: rolldice 4d8*2+2 Roll four six-sided dice, drop the lowest result and add the remain‐ ing results. Do this three times: rolldice 3x4d6s1 HISTORY This version of rolldice was written as an exercise for practicing C. The idea for getnumber() was from an anon from /g/'s dpt. I could've used strtol(3) but, as I said, I did it for practicing. rolldice(6)

Here's the code:

#include <err.h> #include <ctype.h> #include <stdio.h> #include <stdlib.h> #include <limits.h> #include <unistd.h> #include <bsd/stdlib.h> #define DEFROLLS 1 #define DEFDICE 1 #define DEFFACES 6 #define DEFMULTIPLIER 1 #define DEFMODIFIER 0 #define DEFDROP 0 static int separate; /* structure of a dice string */ struct dice { int rolls; int dice; int faces; int multiplier; int modifier; int drop; }; static void rolldice(struct dice); static struct dice getdice(char *); static int getnumber(char **); static void usage(void); /* roll a virtual dice */ int main(int argc, char *argv[]) { struct dice *d; int c, i, exitval; char *line = NULL; size_t linesize = 0; ssize_t linelen; separate = 0; while ((c = getopt(argc, argv, "s")) != -1) { switch (c) { case 's': separate = 1; break; default: usage(); break; } } argc -= optind; argv += optind; exitval = EXIT_SUCCESS; if (argc == 0) { /* no arguments, run interactivelly */ if ((d = reallocarray(NULL, 1, sizeof(*d))) == NULL) err(1, NULL); while ((linelen = getline(&line, &linesize, stdin)) != -1) { *d = getdice(line); if (d->rolls == 0) { warnx("%s: malformed dice string", line); exitval = EXIT_FAILURE; } else { rolldice(*d); } } free(line); if (ferror(stdin)) err(1, "stdin"); } else { /* run parsing the arguments */ if ((d = reallocarray(NULL, argc, sizeof(*d))) == NULL) err(1, NULL); for (i = 0; i < argc; i++) { d[i] = getdice(*argv); if ((d[i]).rolls == 0) errx(1, "%s: malformed dice string", *argv); argv++; } for (i = 0; i < argc; i++) rolldice(d[i]); } free(d); if (ferror(stdout)) err(1, "stdout"); return exitval; } /* get a random roll given a dice structure */ static void rolldice(struct dice d) { int i, j, min, drop; int *roll, rollcount, rollsum; if ((roll = reallocarray(NULL, d.dice, sizeof(*roll))) == NULL) err(1, NULL); rollcount = 1; while (d.rolls-- > 0) { rollsum = 0; if (separate) printf("Roll #%d: (", rollcount++); /* get random values */ for (i = 0; i < d.dice; i++) { roll[i] = 1 + arc4random() % d.faces; rollsum += roll[i]; if (separate) printf("%d%s", roll[i], (i == d.dice-1) ? "" : " "); } /* drop smallest values */ drop = d.drop; while (drop-- > 0) { min = INT_MAX; for (i = 0; i < d.dice; i++) { if (roll[i] != 0 && min > roll[i]) { min = roll[i]; j = i; } } rollsum -= roll[j]; if (separate) printf(" -%d", roll[j]); } /* sum rolls, apply multiplier and modifier */ rollsum = rollsum * d.multiplier + d.modifier; if (separate) { printf(")"); if (d.multiplier != 1) printf(" * %d", d.multiplier); if (d.modifier != 0) printf(" %c %u", (d.modifier < 0) ? '-' : '+', abs(d.modifier)); printf(" = "); } /* print final roll */ printf("%d%c", rollsum, (d.rolls == 0 || separate) ? '\n' : ' '); } free(roll); } /* get dice in format [#x][#]d[#|%][*#][+#|-#][s#], where # is a number */ static struct dice getdice(char *s) { struct dice d; int n, sign; /* set number of rolls */ if ((n = getnumber(&s)) < 0) goto error; d.rolls = DEFROLLS; if (*s == 'x') { d.rolls = (n == 0) ? DEFROLLS : n; s++; if (n < 1) goto error; if ((n = getnumber(&s)) < 0) goto error; } /* set number of dices */ if (*s != 'd') goto error; d.dice = (n == 0) ? DEFDICE : n; n = 0; s++; /* set number of faces */ if (*s == '%') { n = 100; s++; } else if ((n = getnumber(&s)) < 0) goto error; d.faces = (n == 0) ? DEFFACES : n; n = 0; /* set multiplier */ if (*s == '*') { s++; if ((n = getnumber(&s)) < 1) goto error; } d.multiplier = (n == 0) ? DEFMULTIPLIER : n; n = 0; /* set modifier */ if (*s == '+' || *s == '-') { sign = (*s++ == '-') ? -1 : 1; if ((n = getnumber(&s)) < 1) goto error; } d.modifier = (n == 0) ? DEFMODIFIER : sign * n; n = 0; /* set number of drops */ if (*s == 's') { s++; if ((n = getnumber(&s)) < 1) goto error; } d.drop = (n == 0) ? DEFDROP : n; if (d.drop >= d.dice) goto error; if (*s != '\0' && *s != '\n') goto error; return d; error: return (struct dice) {0, 0, 0, 0, 0, 0}; } /* get number from *s; return -1 in case of overflow, return 0 by default */ static int getnumber(char **s) { int n; n = 0; while (isdigit(**s)) { if (n > (INT_MAX - 10) / 10) return -1; else n = n * 10 + **s - '0'; (*s)++; } return n; } static void usage(void) { (void) fprintf(stderr, "usage: rolldice [-s] [dice-string...]\n"); exit(EXIT_FAILURE); }

Is my solution portable? And is it well commented?
I think I rely to much on BSD extensions, such as err(3), and on POSIX extensions, such as getopt(3). It must be compiled with -lbsd on Linux. Is this bad?

Is my code comparable to Stevie's? Or is it worse?
Please, see also Stevie's rolldice, and compare my code to his.

I think Stevie's rolldice contains serious input bugs, such as accepting any string not containing d as 1d6, for example rolldice foo is the same as rolldice 1d6 for Stevie's. And his implementation accepts multiple modifiers, but only uses the last one (rolldice 1d6-3+2+1 is the same as rolldice 1d6+1). My version does not have those bugs.

(Note: I had access to Stevie's rolldice program before writing mine, but I have only seen Stevie's code after finalizing mine, which I wrote from scratch).

EDIT: After reading the answers, I've refactored the code and written a new version of rolldice(6).

chux · Accepted Answer · 2020-03-27 02:38:24Z

Very nicely code.

Only some nits.

Help

"I wrote a manual page for it" --> Perhaps it, or a condensed version for option -h?

 case 'h': printf("blah blah\n); exit (EXIT_SUCCESS);

> vs <

Conceptually, when looking for "smallest values", I'd like to find a <. Perhaps:

 // if (roll[i] != 0 && min > roll[i]) { if (roll[i] != 0 && roll[i] < min) {

Theoretical UB

If for some reason if (roll[i] != 0 && min > roll[i]) is never true, j remains uninitialized and UB later in rollsum -= roll[j];.

Recommend to declare j in the while loop and initialize it.

Theoretical UB

Code relies on modifier never getting/having the value of INT_MIN - which of course it should not with sane input. But let's have some fun.

The 2 lines incurs UB when INT_MIN is involved.

d.modifier = (n == 0) ? DEFMODIFIER : sign * n; printf(" %c %u", (d.modifier < 0) ? '-' : '+', abs(d.modifier));

getnumber(char **s) also has UB potential (n = n * 10 + **s - '0'; can still overflow) with input for values near/above INT_MAX.

Candidate fix/improve for a full range getnumber

// No reason to pass `char **`, pass a `char *` // Moving sign detection here too. static int getnumber(const char *s) { int n = 0; int sign = *s; if (size == '-' || size == '+') s++; // As there are more neg int than pos int, code accumulates on the neg side while (isdigit((unsigned char) *s)) { // Cast to avoid UB of *s < 0 int digit = *s - '0'; if (n <= INT_MIN/10 && (n < INT_MIN/10 || digit > -(INT_MIN%10))} { return -1; } n = n*10 - digit; } if (sign != '-') { if (n < -INT_MAX) { return -1; } n = -n; } return n; }

GTG

The program does support -h. When passing -h (or any other option other than -s) it will show usage(). — phillbush
– phillbush, Commented Mar 27, 2020 at 11:57
@barthooper Yes, I see that now. Detail idea: For getopt(), I like the default to result in a usage() and a prompt EXIT_FAILURE, whereas -h results in usage() and EXIT_SUCCESS. — chux
– chux, Commented Mar 27, 2020 at 12:03
I am following suckless philosophy and OpenBSD practice of considering a dedicated -h bloat, since the program can handle it by default without a special option. Most UNIX programs (except the GNU ones) do not have -h but will show usage() when calling with -h. — phillbush
– phillbush, Commented Mar 27, 2020 at 12:21
Do you think it would be better if I gather all separate's printf at the final of rolldice() and better the readability at the price of adding another, redundant loop? — phillbush
– phillbush, Commented Mar 27, 2020 at 13:28
You should never print usage in response to an incorrect call line. A simple, succinct error message is far better. If you want to treat an incorrect call as a request for usage, then print it to stdout and do not consider it an error. (IMO, that is only appropriate if called with no arguments.) wrp.github.io/blog/2020/02/21/succinct-err-messages.html — William Pursell
– William Pursell, Commented Apr 1, 2020 at 15:23

S.S. Anne · Accepted Answer · 2020-06-10 13:24:26Z

Portability

Your code is mostly portable. I would say to toss the use of BSD extensions. Here's an implementation of reallocarray:

void *reallocarray(void *p, size_t n, size_t sz) { if(n > PTRDIFF_MAX/sz) { /* overflow; avoid undefined behavior for pointer subtractions */ return NULL; } return realloc(p, n * sz); }

For the functions from err.h, you'll have to go in and manually replace those with their equivalents. See also the spec.

I'd say that using POSIX is fine; on Windows, there are plenty of drop-in replacements for getopt and getline.

Consistency

In some places you're using 1 (err, errx), while in others you're using EXIT_FAILURE. If you plan on using POSIX for the long-term, I suggest 1. Otherwise, use EXIT_FAILURE.

Typos

"ommited" should be "omitted".
"dices" should be "dice".

These aren't that big of a problem but bad spelling and grammar are my pet peeve :)

err(1), warn(1) and related functions come in handy and are very convenient, I wonder why they are not specified by POSIX. On the typos, English is not my native language, but I swear they are not that frequent. — phillbush
– phillbush, Commented Apr 10, 2020 at 0:01
@barthooper The typos aren't frequent. You're doing well with spelling and grammar otherwise. — S.S. Anne
– S.S. Anne, Commented Apr 10, 2020 at 0:02

Community · Accepted Answer · 2020-06-10 13:24:26Z

Is it well commented?

Generally yes, the only place I might add more comments is to explain the fields in the structure.

Is my solution portable?

No, it will not port to Windows easily because of the use of libbsd and the use of the unistd.h header file.

To improve portability it is also important to get comfortable with the memory allocation functions void* malloc(size_t size), void* calloc( size_t num, size_t size ) and void *realloc( void *ptr, size_t new_size ). Use the standard C library function char *fgets( char *restrict str, int count, FILE *restrict stream ) instead of getline().

Complexity of the Functions

There are at least 2 sub-functions that could be pulled out of main(), the first is processing the command line arguments, and the second is handling the use input.

The function rolldice(struct dice d) is also too complex, there are 2 or 3 sub-functions in rolldice as well.

There is also a programming principle called the Single Responsibility Principle that applies here. The Single Responsibility Principle states:

that every module, class, or function should have responsibility over a single part of the functionality provided by the software, and that responsibility should be entirely encapsulated by that module, class or function.

What could be a good alternative to libbsd's arc4random? stdlib's random(3) and rand(3) needs to be seeded, if I seed them with srand(time(NULL)), I have to wait a whole second to use rolldice again, since it just reseed after each full second. Is there any way I can reseed it more frequently? — phillbush
– phillbush, Commented Mar 27, 2020 at 11:55
You only need to seed rand once at the beginning of the program. — pacmaninbw
– pacmaninbw ♦, Commented Mar 27, 2020 at 12:37
I know. But when I run the program twice at the same second, both runs will seed the same time(NULL), if I seed it with srand(time(NULL)). Thus generating the same random numbers on each run. — phillbush
– phillbush, Commented Mar 27, 2020 at 12:39

Stack Exchange Network

A dice roller in C

3 Answers 3

Portability

Consistency

Typos

Complexity of the Functions

You must log in to answer this question.

Hot Network Questions

A dice roller in C

3 Answers 3

Portability

Consistency

Typos

Complexity of the Functions

You must log in to answer this question.

Related

Hot Network Questions