On Tue, Oct 30, 2018 at 3:32 AM Denys Vlasenko <vda.li...@googlemail.com> wrote: > > On Tue, Oct 30, 2018 at 12:30 AM Gavin Howard <gavin.d.how...@gmail.com> > wrote: > > On Mon, Oct 29, 2018 at 4:46 PM Denys Vlasenko <vda.li...@googlemail.com> > > wrote: > > > > Because my patch is so large, I cannot get it through the mailing list > > > > filter. Therefore, I have pasted it on pastebin, instead. > > > > > > > > The link is https://pastebin.com/PJa2vaUR and the raw version can be > > > > found at https://pastebin.com/raw/PJa2vaUR > > > > > > Probably could send it bzipped. > > > > I tried, unfortunately. Even though it was under the limit, the > > mailing list software rejected. > > Ok, pastebin would work for the time being. > > > > > + if (!(v->v = malloc(esize * BC_VEC_START_CAP))) return > > > BC_STATUS_ALLOC_ERR; > > > > > > (1) Please do not nest assignments: > > > > > > v->v = malloc(esize * BC_VEC_START_CAP); > > > if (!v->v) > > > return BC_STATUS_ALLOC_ERR; > > > > Oh boy. I was afraid of this. > > > > The problem is that I am also trying to get this bc into toybox (might > > as well reduce duplication of effort), and as I am sure you know, > > Landley is very particular about the line count of commands. To be > > honest, I would prefer your style (except for putting if statements > > that on one line if possible), but this reduces line count. > > > > I don't really know what to do here. > > I can rewrite these constructs after patch is merged. > > > > > +BcStatus bc_vec_pushByte(BcVec *v, uint8_t data) { > > > + return bc_vec_push(v, &data); > > > +} > > > > > > The entire bbox code base uses this formatting of function body: > > > > > > BcStatus bc_vec_pushByte(BcVec *v, uint8_t data) > > > { > > > return bc_vec_push(v, &data); > > > } > > > > This one might be a little harder. > > I can rewrite these constructs after patch is merged. > > > > > + if (!v->len && (s = bc_vec_pushByte(v, '\0'))) return s; > > > > > > len is not boolean or a pointer. !foo is usually used with those. > > > len is an integer. You are testing whether it's zero. Why obfuscate? > > > For readability, should be: > > > if (v->len == 0) { > > > s = bc_vec_pushByte(v, '\0'); > > > if (s) > > > return s; > > > } > > One-line "if (s) return s;" is okay too, when both if-expression and > statement are simple. > > > > > +static void bc_vm_sig(int sig) { > > > + if (sig == SIGINT) { > > > + size_t len = strlen(bcg.sig_msg); > > > + if (write(2, bcg.sig_msg, len) == (ssize_t) len) > > > + bcg.sig += (bcg.signe = bcg.sig == bcg.sigc); > > > + } > > > + else bcg.sig_other = 1; > > > +} > > > > > > + sa.sa_handler = bc_vm_sig; > > > + sa.sa_flags = 0; > > > + > > > + if (sigaction(SIGINT, &sa, NULL) < 0 || sigaction(SIGPIPE, &sa, > > > NULL) < 0 || > > > + sigaction(SIGHUP, &sa, NULL) < 0 || sigaction(SIGTERM, &sa, > > > NULL) < 0) > > > + { > > > + return BC_STATUS_EXEC_SIGACTION_FAIL; > > > + } > > > > > > Calculator has signal handling?! > > > siganction() never fails - no need to check result. > > > > GNU bc has signal handling. It is so users can stop long-running > > calculations. This bc is meant to be compatible. > > Well, without signal handling ^C will stop long-running calculations > too. :) > I can imagine some users who use interactive bc *a lot* (scientist?), > and who would want "^C abort calc but stays in bc" behavior, > but they are likely a minority. Can you make signal handling optional? > > > > Okay, as it stands, I see one big showstopper for getting this into > > busybox: code style. I don't know how to fix code style with a script, > > so if it is a showstopper for you > > It is not.
Thank you for being willing. I can work with this. It may take me a few days to generate a new patch. Gavin Howard _______________________________________________ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox