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 <r...@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@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); > } _______________________________________________ Toybox mailing list Toybox@lists.landley.net http://lists.landley.net/listinfo.cgi/toybox-landley.net