[Toybox] Graff and bc
Gavin Howard
gavin.d.howard at gmail.com
Tue Mar 13 07:49:38 PDT 2018
Rob,
If you inline bc_exec() and bc_vm_init(), I am not sure that I will be
able to maintain this for you. bc_exec() is there just so I could keep
the main function in my repo out of yours. Trying to make that work
with my release script would be killer. I don't think saving 20 out of
9000 lines is worth that, not even for you. Please do not do this
change.
The other changes I can make without a problem.
Gavin Howard
On Tue, Mar 13, 2018 at 8:22 AM, Rob Landley <rob at landley.net> wrote:
> On 03/12/2018 08:41 PM, Rob Landley wrote:> They're both intentionally small and simple, with descriptions in the commit
>> messages. I'll just post the third one to the list...
>
> Next pass, net deletion of 72 lines.
>
> landley at driftwood:~/toybox/toy3$ git diff toys/*/bc.c | diffstat
> bc.c | 174 +++++++++++++++++++------------------------------------------------
> 1 file changed, 51 insertions(+), 123 deletions(-)
>
> Remove the globals bc_code, bc_std, and bc_warn. Just test toys.optflags
> directly at the users instead.
>
> Remove the redundant struct BcGlobals and use the generated struct bc_data
> from generated/globals.h
>
> Inline BC_NUM_ZERO(n) which is just !n->len
>
> BC_NUM_POS_ONE() is only used once and BC_NUM_NEG_ONE() is never used,
> inline the first, delete the second.
>
> Toybox is built with -funsigned-char, so we don't have to say "unsigned char".
> (Avoids spurious type collisions/casts.)
>
> Remove "const" wherever it requires a gratuitous typecast, follow the chain
> back removing "const" until the types stop complaining.
>
> Inline bc_vm_init() and bc_exec() since each is only called once.
>
> Set toys.exitval directly instead of having an intermediate "status" variable.
> (In a future pass this can probably be inlined into the called functions.)
>
> This little trick:
>
> - if (line) fprintf(stderr, ":%d\n\n", line);
> - else fprintf(stderr, "\n\n");
> + fprintf(stderr, ":%d\n\n"+3*!line, line);
>
> STDIN_FILENO and STDOUT_FILENO have been 0 and 1 respectively since 1969.
>
> todo: work out if bc_program_free() and bc_program_free() actually do anything
> other than free memory (which gets freed automatically when we exit). If
> so, they're if (CFG_TOYBOX_FREE) calls at best.
>
> diff --git a/toys/pending/bc.c b/toys/pending/bc.c
> index bd48b02..3ee9705 100644
> --- a/toys/pending/bc.c
> +++ b/toys/pending/bc.c
> @@ -40,10 +40,7 @@ config BC
> #include "toys.h"
>
> GLOBALS(
> - long bc_code;
> long bc_interactive;
> - long bc_std;
> - long bc_warn;
>
> long bc_signal;
> )
> @@ -143,18 +140,6 @@ typedef enum BcStatus {
> typedef void (*BcFreeFunc)(void*);
> typedef BcStatus (*BcCopyFunc)(void*, void*);
>
> -// ** Exclude start.
> -typedef struct BcGlobals {
> -
> - long bc_code;
> - long bc_interactive;
> - long bc_std;
> - long bc_warn;
> -
> - long bc_signal;
> -
> -} BcGlobals;
> -
> void bc_error(BcStatus status);
> void bc_error_file(BcStatus status, const char *file, uint32_t line);
>
> @@ -209,13 +194,8 @@ typedef struct BcNum {
>
> #define BC_NUM_PRINT_WIDTH (69)
>
> -#define BC_NUM_ZERO(n) (!(n)->len)
> -
> #define BC_NUM_ONE(n) ((n)->len == 1 && (n)->rdx == 0 && (n)->num[0] == 1)
>
> -#define BC_NUM_POS_ONE(n) (BC_NUM_ONE(n) && !(n)->neg)
> -#define BC_NUM_NEG_ONE(n) (BC_NUM_ONE(n) && (n)->neg)
> -
> typedef BcStatus (*BcNumUnaryFunc)(BcNum*, BcNum*, size_t);
> typedef BcStatus (*BcNumBinaryFunc)(BcNum*, BcNum*, BcNum*, size_t);
>
> @@ -579,7 +559,7 @@ typedef struct BcLex {
> typedef struct BcLexKeyword {
>
> const char name[9];
> - const unsigned char len;
> + const char len;
> const bool posix;
>
> } BcLexKeyword;
> @@ -758,7 +738,7 @@ typedef struct BcVm {
> BcParse parse;
>
> int filec;
> - const char** filev;
> + char **filev;
>
> } BcVm;
>
> @@ -1169,7 +1149,7 @@ const char *bc_program_ready_prompt = "ready for more input\n\n";
> const char *bc_program_sigint_msg = "\n\ninterrupt (type \"quit\" to exit)\n\n";
> const char *bc_lib_name = "lib.bc";
>
> -const unsigned char bc_lib[] = {
> +const char bc_lib[] = {
> 115,99,97,108,101,61,50,48,10,100,101,102,105,110,101,32,101,40,120,41,123,
> 10,9,97,117,116,111,32,98,44,115,44,110,44,114,44,100,44,105,44,112,44,102,
> 44,118,10,9,98,61,105,98,97,115,101,10,9,105,98,97,115,101,61,65,10,9,105,102,
> @@ -1559,11 +1539,11 @@ int bc_num_compareDigits(BcNum *a, BcNum *b, size_t *digits) {
> }
> else if (b->neg) return 1;
>
> - if (BC_NUM_ZERO(a)) {
> + if (!a->len) {
> cmp = b->neg ? 1 : -1;
> - return BC_NUM_ZERO(b) ? 0 : cmp;
> + return !b->len ? 0 : cmp;
> }
> - else if (BC_NUM_ZERO(b)) return a->neg ? -1 : 1;
> + else if (!b->len) return a->neg ? -1 : 1;
>
> a_int = a->len - a->rdx;
> b_int = b->len - b->rdx;
> @@ -1820,12 +1800,12 @@ BcStatus bc_num_alg_s(BcNum *a, BcNum *b, BcNum *c, size_t sub) {
> // I am hijacking it to tell this function whether it is doing an add
> // or a subtract.
>
> - if (BC_NUM_ZERO(a)) {
> + if (!a->len) {
> status = bc_num_copy(c, b);
> c->neg = !b->neg;
> return status;
> }
> - else if (BC_NUM_ZERO(b)) return bc_num_copy(c, a);
> + else if (!b->len) return bc_num_copy(c, a);
>
> aneg = a->neg;
> bneg = b->neg;
> @@ -1894,7 +1874,7 @@ BcStatus bc_num_alg_m(BcNum *a, BcNum *b, BcNum *c, size_t scale) {
> size_t j;
> size_t len;
>
> - if (BC_NUM_ZERO(a) || BC_NUM_ZERO(b)) {
> + if (!a->len || !b->len) {
> bc_num_zero(c);
> return BC_STATUS_SUCCESS;
> }
> @@ -1960,8 +1940,8 @@ BcStatus bc_num_alg_d(BcNum *a, BcNum *b, BcNum *c, size_t scale) {
> size_t i;
> BcNum copy;
>
> - if (BC_NUM_ZERO(b)) return BC_STATUS_MATH_DIVIDE_BY_ZERO;
> - else if (BC_NUM_ZERO(a)) {
> + if (!b->len) return BC_STATUS_MATH_DIVIDE_BY_ZERO;
> + else if (!a->len) {
> bc_num_zero(c);
> return BC_STATUS_SUCCESS;
> }
> @@ -2151,7 +2131,7 @@ BcStatus bc_num_alg_p(BcNum *a, BcNum *b, BcNum *c, size_t scale) {
> bc_num_one(c);
> return BC_STATUS_SUCCESS;
> }
> - else if (BC_NUM_ZERO(a)) {
> + else if (!a->len) {
> bc_num_zero(c);
> return BC_STATUS_SUCCESS;
> }
> @@ -2274,11 +2254,12 @@ BcStatus bc_num_sqrt_newton(BcNum *a, BcNum *b, size_t scale) {
> size_t resrdx;
> int cmp;
>
> - if (BC_NUM_ZERO(a)) {
> + if (!a->len) {
> bc_num_zero(b);
> return BC_STATUS_SUCCESS;
> }
> - else if (BC_NUM_POS_ONE(a)) {
> + else if (BC_NUM_ONE(a) && !a->neg) {
> +
> bc_num_one(b);
> return bc_num_extend(b, scale);
> }
> @@ -2864,7 +2845,7 @@ BcStatus bc_num_printBase(BcNum *n, BcNum *base, size_t base_t, FILE* f) {
>
> if (status) goto frac_len_err;
>
> - while (!BC_NUM_ZERO(&intp)) {
> + while (intp.len) {
>
> unsigned long dig;
>
> @@ -3066,7 +3047,7 @@ BcStatus bc_num_fprint(BcNum *n, BcNum *base, size_t base_t,
> if (base_t < BC_NUM_MIN_BASE || base_t > BC_NUM_MAX_OUTPUT_BASE)
> return BC_STATUS_EXEC_INVALID_OBASE;
>
> - if (BC_NUM_ZERO(n)) {
> + if (!n->len) {
> if (fputc('0', f) == EOF) return BC_STATUS_IO_ERR;
> status = BC_STATUS_SUCCESS;
> }
> @@ -8759,7 +8740,7 @@ BcStatus bc_vm_execFile(BcVm *vm, int idx) {
> char *data;
> BcProgramExecFunc exec;
>
> - exec = TT.bc_code ? bc_program_print : bc_program_exec;
> + exec = (toys.optflags&FLAG_i) ? bc_program_print : bc_program_exec;
>
> file = vm->filev[idx];
> vm->program.file = file;
> @@ -9050,7 +9031,7 @@ BcStatus bc_vm_execStdin(BcVm *vm) {
>
> if (BC_PARSE_CAN_EXEC(&vm->parse)) {
>
> - if (!TT.bc_code) {
> + if (!(toys.optflags&FLAG_i)) {
>
> status = bc_program_exec(&vm->program);
>
> @@ -9113,33 +9094,6 @@ buf_err:
> return status;
> }
>
> -BcStatus bc_vm_init(BcVm *vm, int filec, const char *filev[]) {
> -
> - BcStatus status;
> - struct sigaction act;
> -
> - sigemptyset(&act.sa_mask);
> - act.sa_handler = bc_vm_sigint;
> -
> - if (sigaction(SIGINT, &act, NULL) < 0) return BC_STATUS_EXEC_SIGACTION_FAIL;
> -
> - status = bc_program_init(&vm->program);
> -
> - if (status) return status;
> -
> - status = bc_parse_init(&vm->parse, &vm->program);
> -
> - if (status) {
> - bc_program_free(&vm->program);
> - return status;
> - }
> -
> - vm->filec = filec;
> - vm->filev = filev;
> -
> - return BC_STATUS_SUCCESS;
> -}
> -
> void bc_vm_free(BcVm *vm) {
> bc_parse_free(&vm->parse);
> bc_program_free(&vm->program);
> @@ -9206,92 +9160,66 @@ void bc_error_file(BcStatus status, const char *file, uint32_t line) {
> bc_err_descs[status]);
>
> fprintf(stderr, " %s", file);
> -
> - if (line) fprintf(stderr, ":%d\n\n", line);
> - else fprintf(stderr, "\n\n");
> + fprintf(stderr, ":%d\n\n"+3*!line, line);
> }
>
> BcStatus bc_posix_error(BcStatus status, const char *file,
> uint32_t line, const char *msg)
> {
> - if (!(TT.bc_std || TT.bc_warn) ||
> - status < BC_STATUS_POSIX_NAME_LEN ||
> - !file)
> - {
> + int s = toys.optflags & FLAG_s, w = toys.optflags & FLAG_w;
> +
> + if (!(s || w) || status<BC_STATUS_POSIX_NAME_LEN || !file)
> return BC_STATUS_SUCCESS;
> - }
>
> fprintf(stderr, "\n%s %s: %s\n", bc_err_types[status],
> - TT.bc_std ? "error" : "warning", bc_err_descs[status]);
> + s ? "error" : "warning", bc_err_descs[status]);
>
> if (msg) fprintf(stderr, " %s\n", msg);
>
> fprintf(stderr, " %s", file);
> + fprintf(stderr, ":%d\n\n"+3*!line, line);
>
> - if (line) fprintf(stderr, ":%d\n\n", line);
> - else fprintf(stderr, "\n\n");
> -
> - return TT.bc_std ? status : BC_STATUS_SUCCESS;
> + return status*!!s;
> }
>
> -BcStatus bc_exec(unsigned int flags, unsigned int filec, const char *filev[]) {
> -
> - BcStatus status;
> +void bc_main(void)
> +{
> + struct sigaction act;
> BcVm vm;
>
> - if ((flags & FLAG_i) || (isatty(STDIN_FILENO) && isatty(STDOUT_FILENO))) {
> - TT.bc_interactive = 1;
> - } else TT.bc_interactive = 0;
> + TT.bc_interactive = (toys.optflags & FLAG_i) || (isatty(0) && isatty(1));
>
> - TT.bc_code = flags & FLAG_i;
> - TT.bc_std = flags & FLAG_s;
> - TT.bc_warn = flags & FLAG_w;
> + if (!(toys.optflags & FLAG_q) && (toys.exitval = bc_print_version())) return;
>
> - if (!(flags & FLAG_q)) {
> -
> - status = bc_print_version();
> -
> - if (status) return status;
> + sigemptyset(&act.sa_mask);
> + act.sa_handler = bc_vm_sigint;
> + if (sigaction(SIGINT, &act, NULL) < 0) {
> + toys.exitval = BC_STATUS_EXEC_SIGACTION_FAIL;
> + return;
> + }
> + if ((toys.exitval = bc_program_init(&vm.program))) return;
> + if ((toys.exitval = bc_parse_init(&vm.parse, &vm.program))) {
> + bc_program_free(&vm.program);
> + return;
> }
>
> - status = bc_vm_init(&vm, filec, filev);
> -
> - if (status) return status;
> -
> - if (flags & FLAG_l) {
> -
> - status = bc_parse_file(&vm.parse, bc_lib_name);
> -
> - if (status) goto err;
> -
> - status = bc_parse_text(&vm.parse, (const char*) bc_lib);
> -
> - if (status) goto err;
> + vm.filec = toys.optc;
> + vm.filev = toys.optargs;
>
> - while (!status) status = bc_parse_parse(&vm.parse);
> + if (toys.optflags & FLAG_l) {
> + if ((toys.exitval = bc_parse_file(&vm.parse, bc_lib_name))
> + || (toys.exitval = bc_parse_text(&vm.parse, bc_lib))) goto err;
> + do toys.exitval = bc_parse_parse(&vm.parse); while (!toys.exitval);
>
> - if (status != BC_STATUS_LEX_EOF && status != BC_STATUS_PARSE_EOF) goto err;
> + if (toys.exitval!=BC_STATUS_LEX_EOF && toys.exitval!=BC_STATUS_PARSE_EOF)
> + goto err;
>
> // Make sure to execute the math library.
> - status = bc_program_exec(&vm.program);
> -
> - if (status) goto err;
> + if ((toys.exitval = bc_program_exec(&vm.program))) goto err;
> }
>
> - status = bc_vm_exec(&vm);
> + toys.exitval = bc_vm_exec(&vm);
>
> err:
> -
> bc_vm_free(&vm);
> -
> - return status;
> -}
> -
> -void bc_main(void) {
> -
> - unsigned int flags;
> -
> - flags = (unsigned int) toys.optflags;
> -
> - toys.exitval = (char) bc_exec(flags, toys.optc, (const char**) toys.optargs);
> }
More information about the Toybox
mailing list