[Toybox] Graff and bc

Gavin Howard gavin.d.howard at gmail.com
Tue Mar 13 08:05:29 PDT 2018


Also, bc_code is equivalent to "toys.optflags & FLAG_c", not
"toys.optflags & FLAG_i." FLAG_i is for bc_interactive.
GH

On Tue, Mar 13, 2018 at 8:49 AM, Gavin Howard <gavin.d.howard at gmail.com> wrote:
> 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