Re: dc's stack breaks when doing modulo-0

2019-05-08 Thread Gavin Howard
> dc -e '4 0 % p' mess up the stack so bad that my MIPS kernel traps it and 
> reboots.
>
> I think we should, like the regular dc, check for 0 as a divider when doing 
> modulo.
>
> //M

Markus,

Was this the full dc or the small dc? As far as I know (I am the
original author of the full dc), the full dc does, in fact, return an
error and not trap.

Gavin Howard
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


Re: [git commit] bc: new applet, throws warning

2018-12-07 Thread Gavin Howard
Sure, it's 10 times bigger, but it actually implements arbitrary-precision
math, implements all of the dc commands except 1, and (this is probably the
biggest thing) implements dc strings properly, including being able to
*execute* them, conditionally and unconditionally.

I personally feel like it's an apples-to-oranges comparison.

That said, if you want my code out of busybox, just let me know. That's
your prerogative, and I would be wrong to be upset.

Gavin Howard

On Fri, Dec 7, 2018, 01:12 Bernhard Reutner-Fischer  On 6 December 2018 22:38:45 CET, Gavin Howard 
> wrote:
> >On Thu, Dec 6, 2018 at 2:30 PM Michael Conrad 
> >wrote:
> >>
> >> On 12/6/2018 11:48 AM, Gavin Howard wrote:
> >>
> >> > you are going to have to make the bc not give good error messages
> >and/or not check for errors as thoroughly (a massive chunk of the
> >parser, which is the largest portion, is dedicated to error checking),
> >reduce the quality of the code, reduce the performance of the math
> >(though this would not remove much), or all of them combined.
> >>
> >> Just FYI, these are typical things people do for busybox applets ;-)
> >>
> >> I'm not a bc user, so I don't care either way; having the applet
> >seems better than not having it, and you seem to have thoroughly
> >completed a difficult project.  …but I do think maybe you missed the
> >spirit of Busybox code.
> >
> >Fair enough, mostly, because I know the point of busybox, actually. I
>
> Don't get me wrong, but given your dc was   10 (!) times the size of our
> existing  implementation I would be surprised if the bc written in the same
> style was as small as it should be.
>
> thanks,
>
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


Re: [git commit] bc: new applet, throws warning

2018-12-06 Thread Gavin Howard
On Thu, Dec 6, 2018 at 2:30 PM Michael Conrad  wrote:
>
> On 12/6/2018 11:48 AM, Gavin Howard wrote:
>
> > you are going to have to make the bc not give good error messages and/or 
> > not check for errors as thoroughly (a massive chunk of the parser, which is 
> > the largest portion, is dedicated to error checking), reduce the quality of 
> > the code, reduce the performance of the math (though this would not remove 
> > much), or all of them combined.
>
> Just FYI, these are typical things people do for busybox applets ;-)
>
> I'm not a bc user, so I don't care either way; having the applet seems better 
> than not having it, and you seem to have thoroughly completed a difficult 
> project.  …but I do think maybe you missed the spirit of Busybox code.

Fair enough, mostly, because I know the point of busybox, actually. I
have given the maintainers the option of doing whatever they wish with
my bc; it's just that if they do, I probably can't help maintain it.

That may not be a bad thing, either, since I have written this bc to
basically be perfect, so it will probably not need any maintenance at
all.

Gavin Howard
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


Re: [git commit] bc: new applet, throws warning

2018-12-06 Thread Gavin Howard
Hello,

I am the original bc author, so take my words with a grain of salt.

Of course, I will understand if busybox decides to write its own
implementation, I just wouldn't suggest it.

First of all, it is important to remember everything that must be in a
bc implementation:

1) Lexer
2) Parser
3) Code generator (mixed with the parser in my implementation)
4) Interpreter
5) Arbitrary precision math library
6) Primitive garbage collection (if you want to not have memory leaks)

I did all that, plus a complete dc (which has its own lexer, parser,
and parts of the interpreter), while making it free of memory leaks
and memory errors, in under 7500 LOC. That is no mean feat. And I did
it while providing more detailed error messages than the GNU bc, while
being about two-thirds the size when compiled (standalone). Inside
busybox, this bc (combined with dc) is about the size of the GNU dc
alone.

To reduce the size significantly (Denys has reduced it some already,
though in my mind, some of those changes are up in the air), you are
going to have to make the bc not give good error messages and/or not
check for errors as thoroughly (a massive chunk of the parser, which
is the largest portion, is dedicated to error checking), reduce the
quality of the code, reduce the performance of the math (though this
would not remove much), or all of them combined.

I think the best idea would be to accept my code (with changes), but
just not enable it by default. Users can choose if they want it.

Gavin Howard
On Thu, Dec 6, 2018 at 6:37 AM Bernhard Reutner-Fischer
 wrote:
>
> Hi,
>
> Are you sure this is a good idea?
> I was under the impression that it would be much more sane to write bc from 
> scratch given the extraordinary size of the proposed implementation.
>
> Thanks,
>
> On 5 December 2018 15:40:38 CET, Denys Vlasenko  
> wrote:
> >commit:
> >https://git.busybox.net/busybox/commit/?id=01055ba89a92470587d9012c7e8707d943ebd875
> >branch: https://git.busybox.net/busybox/commit/?id=refs/heads/master
> >
> >Signed-off-by: Gavin Howard 
> >Signed-off-by: Denys Vlasenko 
> >---
> >miscutils/bc.c | 7449
> >
> > miscutils/dc.c |   84 +-
> > 2 files changed, 7492 insertions(+), 41 deletions(-)
> >
> >Patch is too large, so refusing to show it
> >___
> >busybox-cvs mailing list
> >busybox-...@busybox.net
> >http://lists.busybox.net/mailman/listinfo/busybox-cvs
>
> ___
> busybox mailing list
> busybox@busybox.net
> http://lists.busybox.net/mailman/listinfo/busybox
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


Re: [PATCH] bc Version 1.1

2018-12-04 Thread Gavin Howard
Whoops. I had a bug in the code I just sent. The correct code is:

+if (prev == BC_INST_BOOL_NOT || nexprs != 1)
+return BC_STATUS_PARSE_BAD_EXP;

+for (i = 0; i < next.len && t != next.tokens[i]; ++i) continue;
+if (i == next.len) return BC_STATUS_PARSE_BAD_EXP;

Gavin Howard
On Tue, Dec 4, 2018 at 1:48 PM Gavin Howard  wrote:
>
> I think a possible alternative would be:
>
> +if (prev == BC_INST_BOOL_NOT || nexprs != 1) return s;
>
> +for (i = 0; i < next.len && t != next.tokens[i]; ++i) continue;
> +if (i == next.len) return BC_STATUS_PARSE_BAD_EXP;
>
> Is that what you would like?
>
> Gavin Howard
>
> On Tue, Dec 4, 2018 at 1:33 PM Denys Vlasenko  
> wrote:
> >
> > On Sat, Nov 3, 2018 at 6:17 PM Gavin Howard  
> > wrote:
> > >
> > > Hello,
> > >
> > > After making changes to the bc for Denys' requests, I have a better
> > > version of the bc.
> > >
> > > This version, pasted at https://pastebin.com/0M9sMhtM and raw at
> > > https://pastebin.com/raw/0M9sMhtM, has fulfilled every one of Denys'
> > > requests except for a few.
> >
> >
> > +s = BC_STATUS_PARSE_BAD_EXP;
> > +if (prev == BC_INST_BOOL_NOT || nexprs != 1) return s;
> > +
> > +for (i = 0; s && i < next.len; ++i) s *= t != next.tokens[i];
> > +if (s) return s;
> >
> > This is not readable
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


Re: [PATCH] bc Version 1.1

2018-12-04 Thread Gavin Howard
I think a possible alternative would be:

+if (prev == BC_INST_BOOL_NOT || nexprs != 1) return s;

+for (i = 0; i < next.len && t != next.tokens[i]; ++i) continue;
+if (i == next.len) return BC_STATUS_PARSE_BAD_EXP;

Is that what you would like?

Gavin Howard

On Tue, Dec 4, 2018 at 1:33 PM Denys Vlasenko  wrote:
>
> On Sat, Nov 3, 2018 at 6:17 PM Gavin Howard  wrote:
> >
> > Hello,
> >
> > After making changes to the bc for Denys' requests, I have a better
> > version of the bc.
> >
> > This version, pasted at https://pastebin.com/0M9sMhtM and raw at
> > https://pastebin.com/raw/0M9sMhtM, has fulfilled every one of Denys'
> > requests except for a few.
>
>
> +s = BC_STATUS_PARSE_BAD_EXP;
> +if (prev == BC_INST_BOOL_NOT || nexprs != 1) return s;
> +
> +for (i = 0; s && i < next.len; ++i) s *= t != next.tokens[i];
> +if (s) return s;
>
> This is not readable
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


git clone fail

2018-12-03 Thread Gavin Howard
Hello,

After the most recent commit
(d08206dce1291f512d7de9037d9ef1ffbf705cac), git clone is failing as
follows:

$ git clone https://git.busybox.net/busybox

Cloning into 'busybox'...
error: garbage at end of loose object 'ca908973beb26bda7f120362eb942d43c6cfb4c5'
fatal: loose object ca908973beb26bda7f120362eb942d43c6cfb4c5 (stored
in busybox/.git/objects/ca/908973beb26bda7f120362eb942d43c6cfb4c5) is
corrupt
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


Re: [PATCH] bc Version 1.1

2018-12-01 Thread Gavin Howard
I have an updated patch for the comments: https://pastebin.com/ZCvPitQ0

Raw is at: https://pastebin.com/raw/ZCvPitQ0

> Sorry for the delay in reviewing.
>
> Let's take a look at bc_vm_printf(). Basically:
> bad = vfprintf(f, fmt, args) < 0;
> if (bad) bc_vm_exit(BC_STATUS_IO_ERR);
>
> What does bc_vm_exit() do? -
> static void bc_vm_exit(BcStatus s) {
> bc_vm_printf(stderr, bc_err_fmt, bc_errs[bc_err_ids[s]],
> bc_err_msgs[s]);
> exit((int) s);
> }
>
> This would enter infinite loop if you bc_vm_printf(stderr) and meet
> IO error.

Agreed. That line is hard to test. Sorry that I missed that obvious
mistake when adding xmalloc support. It has been fixed in the patch in
this email.

> > $ $BC_ROOT/tests/all.sh bc $BUSYBOX_ROOT/busybox bc
>
> Trying does this output means tests failed or succeeded?? -

Succeeded. The important bit is what the return value ("$?") is. If it
returned 0, no errors. Non-zero: an error.

The reason for this is because the test suite also tests that the bc
will not crash when given invalid input. Obviously, error messages
will be printed in that case, but it means that the bc is working
well.

> In fact, all calls of bc_vm_exit() are called with
> s == BC_STATUS_IO_ERR.
>
> Thus, bc_errs[bc_err_ids[s]], bc_err_msgs[s] are constants.

Changed. Be aware that I still used the arrays because they are used
for other statuses, so it was better to keep the strings in the
arrays.

By the way, the original reason it was this way was because in order
to call xmalloc and friends, I had to implement versions for the
non-busybox bc, and they call bc_vm_exit as well, making the parameter
necessary in the standalone version of bc. However, I couldn't find
analogous busybox versions of bc_vm_printf and friends, so I had to
keep those in the busybox version. In the original patch, I just
didn't remove the parameters.

> static void bc_vm_info(const char *const help)
> {
> printf("%s "BB_VER"\n", applet_name);
> fputs(bc_copyright, stdout);
> if (help) printf(help, applet_name);
> }
>
> In all callsites, help is NULL.

Changed in the patch to your BB_VER print statement, and the help is removed.

>return s == BC_STATUS_QUIT ? BC_STATUS_SUCCESS : s;
>
> You can write it in more straightforward way:
>
>if (s == BC_STATUS_QUIT)
>s = BC_STATUS_SUCCESS;
>return s;

I must admit that I disagree, but it is changed in the patch.

To make things easier on you for reviewing the changes in the patch,
which do include changes that I made myself in the time between my
last submission and now, I have added a patch below to show the
changes from the previous version. This patch is not valid; it's only
for your convenience.

Also, I have put the new output for all 3 make bloatcheck runs below the patch.

>From 519962511fec6e1794a79cdab8b3a0a3529b11c0 Mon Sep 17 00:00:00 2001
From: Gavin Howard 
Date: Sat, 1 Dec 2018 18:08:28 -0700
Subject: [PATCH] Update patch

---
 miscutils/bc.c | 113 -
 1 file changed, 55 insertions(+), 58 deletions(-)

diff --git a/miscutils/bc.c b/miscutils/bc.c
index 4969cd9cd..007d2cea5 100644
--- a/miscutils/bc.c
+++ b/miscutils/bc.c
@@ -7,7 +7,7 @@
  * **Do not edit unless you know what you are doing. **
  */
 //config:config BC
-//config: bool "bc (45.73 kb; 49.84 kb when combined with dc)"
+//config: bool "bc (45.77 kb; 49.88 kb when combined with dc)"
 //config: default n
 //config: help
 //config: bc is a command-line, arbitrary-precision calculator with a
@@ -58,7 +58,7 @@
 //config: enabled.
 //config:
 //config:config DC
-//config: bool "dc (38.28 kb; 49.84 kb when combined with bc)"
+//config: bool "dc (38.32 kb; 49.88 kb when combined with bc)"
 //config: default n
 //config: help
 //config: dc is a reverse-polish notation command-line calculator which
@@ -168,7 +168,7 @@

 typedef enum BcStatus {

- BC_STATUS_SUCCESS,
+ BC_STATUS_SUCCESS = 0,

  BC_STATUS_ALLOC_ERR,
  BC_STATUS_IO_ERR,
@@ -234,11 +234,11 @@ typedef enum BcStatus {
  BC_STATUS_POSIX_BRACE,
 #endif // ENABLE_BC

+ BC_STATUS_INVALID_OPTION,
+
  BC_STATUS_QUIT,
  BC_STATUS_LIMITS,

- BC_STATUS_INVALID_OPTION,
-
 } BcStatus;

 #define BC_ERR_IDX_VM (0)
@@ -697,7 +697,7 @@ typedef struct BcParse {

 #ifdef ENABLE_BC

-BcStatus bc_main(int argc, char *argv[]);
+int bc_main(int argc, char *argv[]);

 typedef struct BcLexKeyword {
  const char name[9];
@@ -731,7 +731,7 @@ static BcStatus bc_parse_expr(BcParse *p, uint8_t
flags, BcParseNext next);

 #define DC_PARSE_BUF_LEN ((int) (sizeof(uint32_t) * CHAR_BIT))

-BcStatus dc_main(int argc, char *argv[]);
+int dc_main(int argc, char *argv[]);

 static BcStatus dc_lex_token(BcLex *l);

@@ -849,11 +849,11 @@ typedef s

[PATCH] bc Version 1.1

2018-11-03 Thread Gavin Howard
Hello,

After making changes to the bc for Denys' requests, I have a better
version of the bc.

This version, pasted at https://pastebin.com/0M9sMhtM and raw at
https://pastebin.com/raw/0M9sMhtM, has fulfilled every one of Denys'
requests except for a few.

First, I still did not use read_line_input. The reason for this is
because bc_read_line does some important error checking. I did,
however, #if guard signal handling and add a config for it. And I made
my signal handling function less buggy (in a single-threaded
environment).

I used getopt32 (and getopt32long behind a config option). I changed
CONFIG_foo to ENABLE_foo. I made everything but bc_main and dc_main
static. I called xmalloc_open_read_close in bc_read_file, but still
kept error handling.

I also tried to pull nested assignments out where I could, but there
were a few places where doing so introduced bugs. That said, all of
them are out (as far as I can tell), unless they could not be, so I
would not mess with them any further if I were you.

On that note, if you do mess with them, you want the full test suite
while reviewing this code, which you can get by cloning my repo at
https://github.com/gavinhoward/bc and then run the following commands:

$ $BC_ROOT/tests/all.sh bc $BUSYBOX_ROOT/busybox bc
$ $BC_ROOT/tests/all.sh dc $BUSYBOX_ROOT/busybox dc

If you also want to test the Linux kernel timeconst script, you can run:

$ $BC_ROOT/tests/bc/timeconst.sh ./path/to/timeconst.bc
$BUSYBOX_ROOT/busybox bc

I changed my code to call xmalloc and friends, which is actually what
allowed me to pull out assignments. The code ended up *much* smaller
than what I have already given to Landley for toybox, so I tried
pulling them out, and when I did, the code was *still* smaller. We'll
hope that is good enough for him. Also, because of clang-format, I was
able to change the braces in my release script, thus making it work
for both toybox and busybox.

With that said, after testing this thing as hard as I can, I am
supremely confident in it, and I believe that, barring any small bugs,
it is feature-complete and basically finished. Because of that, I am
willing to hand it completely over to busybox, if the maintainers so
desire, although I am willing to support it directly, as well as share
responsibility. I am good with whatever.

Thank you for your patience to this point.

Gavin Howard
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


Re: [PATCH] Add bc (Last patch)

2018-10-30 Thread Gavin Howard
On Tue, Oct 30, 2018 at 3:32 AM Denys Vlasenko  wrote:
>
> On Tue, Oct 30, 2018 at 12:30 AM Gavin Howard  
> wrote:
> > On Mon, Oct 29, 2018 at 4:46 PM Denys Vlasenko  
> > 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, );
> > > +}
> > >
> > > 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, );
> > > }
> >
> > 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, , NULL) < 0 || sigaction(SIGPIPE, ,
> > > NULL) < 0 ||
> > > +sigaction(SIGHUP, , NULL) < 0 || sigaction(SIGTERM, , 
> > > 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


Re: [PATCH] Add bc (Last patch)

2018-10-29 Thread Gavin Howard
On Mon, Oct 29, 2018 at 4:46 PM Denys Vlasenko  wrote:
>
> On Mon, Oct 29, 2018 at 4:55 PM Gavin Howard  wrote:
> >
> > Hello,
> >
> > 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.

> All functions except bc_main() should be static.
> Check with nm --size-sort bc.o

I will see what I can do about that.

> Use : the entire
>
> +BcStatus bc_read_file(const char *path, char **buf) {
> +
> +BcStatus s = BC_STATUS_IO_ERR;
> +FILE *f;
> +size_t size, read;
> +long res;
> +struct stat pstat;
> +
> +if (!(f = fopen(path, "r"))) return BC_STATUS_EXEC_FILE_ERR;
> +
> +if (fstat(fileno(f), ) == -1) goto malloc_err;
> +
> +if (S_ISDIR(pstat.st_mode)) {
> +s = BC_STATUS_PATH_IS_DIR;
> +goto malloc_err;
> +}
> +
> +if (fseek(f, 0, SEEK_END) == -1) goto malloc_err;
> +if ((res = ftell(f)) < 0) goto malloc_err;
> +if (fseek(f, 0, SEEK_SET) == -1) goto malloc_err;
> +
> +if (!(*buf = malloc((size = (size_t) res) + 1))) {
> +s = BC_STATUS_ALLOC_ERR;
> +goto malloc_err;
> +}
> +
> +if ((read = fread(*buf, 1, size, f)) != size) goto read_err;
>
> can be replaced by single xmalloc_open_read_close()!

Will do.

> (The entire
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> should be replaced by
> #include "libbb.h"
> #include 
> )

Sorry about that. I tried to follow the documentation as exactly as I could.

> +while ((c = getopt_long(argc, argv, bc_args_opt, bc_args_lopt,
> )) != -1)
> +{
>
> Use getopt32() ?

Okay. Will do.

> +typedef struct BcResult {
> +
> +BcResultType t;
> +BcResultData d;
> +
> +} BcResult;
>
> Lets drop these extra empty lines.

Will do.

> +#ifdef CONFIG_BC
> +BcStatus bc_vm_posixError(BcStatus s, const char *file,
> +  size_t line, const char *msg);
> +#endif // CONFIG_BC
>
> "ifdef CONFIG_foo" is unsafe versus typos. Use "if ENABLE_foo" - it
> gives warnings
> for typos.

Oh? Did not know that.

> +#ifdef CONFIG_BC
> +
> +#endif // CONFIG_BC
>
> Why these empty ifdef blocks?

This was caused by bugs in my release script that have been fixed.

> +const char bc_name[] = "bc";
>
> static
>
>
> +const char *bc_errs[] = {
>
> static const char *const bc_errs[] = {

Will do.

> +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.

> (2) Use xmalloc/xzalloc/xrealloc which never return NULL.
> We do not try to survive out-of-memory. No check is necessary.

Will do. I was not aware of those functions. bc does have to print a
diagnostic message (see
http://pubs.opengroup.org/onlinepubs/9699919799/utilities/bc.html#tag_20_09_15),
but I checked that those procedures do that.

> +BcStatus bc_vec_pushByte(BcVec *v, uint8_t data) {
> +return bc_vec_push(v, );
> +}
>
> 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, );
> }

This one might be a little harder.

> +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;
> }
>
>
> +if (a == b) return 0;
> +else if (!a->len) return BC_NUM_NEG(!!b->len, !b->neg);
> +else if (!b->

Re: bc

2018-10-29 Thread Gavin Howard
Just sent it today, actually. Thank you.
Gavin H.
On Mon, Oct 29, 2018 at 4:05 PM Denys Vlasenko  wrote:
>
> On Fri, Oct 12, 2018 at 1:11 AM Gavin Howard  wrote:
> > Hello,
> >
> > In March, I wrote
> > (http://lists.busybox.net/pipermail/busybox/2018-March/086300.html)
> > asking if there was interest in having my bc
> > (https://github.com/gavinhoward/bc) in busybox. I got some interest
> > back then.
> >
> > Well, it is ready. Do the maintainers still want it?
>
> No need to ask. Just send the patch for review.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


[PATCH] Add bc (Last patch)

2018-10-29 Thread Gavin Howard
Hello,

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

I look forward to working with the maintainers on getting this added.
(And sorry for the moderator approval spam.)

Gavin Howard
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


bc

2018-10-13 Thread Gavin Howard
I should also mention that I can remove the dc from my patch, if that ends
up being what the maintainers want.

Gavin Howard

On Thu, Oct 11, 2018, 17:11 Gavin Howard  wrote:

> Hello,
>
> In March, I wrote
> (http://lists.busybox.net/pipermail/busybox/2018-March/086300.html)
> asking if there was interest in having my bc
> (https://github.com/gavinhoward/bc) in busybox. I got some interest
> back then.
>
> Well, it is ready. Do the maintainers still want it?
>
> Sorry, it took so long; I was waiting for an opportunity to push it to
> a very high quality before releasing as 1.0. The wait is worth it,
> though, because the quality is second to none. I have fuzzed it until
> it just won't crash and has absolutely zero memory errors or leaks. I
> have run hundreds of thousands of random math tests to make sure it is
> correct. You will also be getting a complete dc (besides the "!"
> command because that's dangerous) because I added that as well (and
> tested it just as well). Also, it does work for the Linux kernel
> build.
>
> Note: I am also not sending a patch in this email because it will be
> *HUGE*. bc is complex, so it is 46.59 kb by "make bloatcheck". dc is
> 36.64 kb, and together, they are 56.35 kb. That is smaller than the
> GNU bc alone, so I don't think it's too bad, but I admit that it is a
> lot.
>
> When I release 1.0 (and I am in the release process right now; I am
> going through a thorough bunch of checks including POSIX compliance),
> busybox will get the patch, if desired. In fact, I can send a patch
> now, so it can be evaluated and possibly be accepted into "make
> defconfig" (unless size is a dealbreaker for that) when I *do* release
> 1.0. I already have the script written to generate the busybox
> version, so it will literally be a matter of running the script,
> running the test suite (which I have already done, actually), running
> git patch, compressing it (like I said, *HUGE*), and sending it.
>
> Thank you for your time.
>
> Gavin Howard
>
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


bc

2018-10-11 Thread Gavin Howard
Hello,

In March, I wrote
(http://lists.busybox.net/pipermail/busybox/2018-March/086300.html)
asking if there was interest in having my bc
(https://github.com/gavinhoward/bc) in busybox. I got some interest
back then.

Well, it is ready. Do the maintainers still want it?

Sorry, it took so long; I was waiting for an opportunity to push it to
a very high quality before releasing as 1.0. The wait is worth it,
though, because the quality is second to none. I have fuzzed it until
it just won't crash and has absolutely zero memory errors or leaks. I
have run hundreds of thousands of random math tests to make sure it is
correct. You will also be getting a complete dc (besides the "!"
command because that's dangerous) because I added that as well (and
tested it just as well). Also, it does work for the Linux kernel
build.

Note: I am also not sending a patch in this email because it will be
*HUGE*. bc is complex, so it is 46.59 kb by "make bloatcheck". dc is
36.64 kb, and together, they are 56.35 kb. That is smaller than the
GNU bc alone, so I don't think it's too bad, but I admit that it is a
lot.

When I release 1.0 (and I am in the release process right now; I am
going through a thorough bunch of checks including POSIX compliance),
busybox will get the patch, if desired. In fact, I can send a patch
now, so it can be evaluated and possibly be accepted into "make
defconfig" (unless size is a dealbreaker for that) when I *do* release
1.0. I already have the script written to generate the busybox
version, so it will literally be a matter of running the script,
running the test suite (which I have already done, actually), running
git patch, compressing it (like I said, *HUGE*), and sending it.

Thank you for your time.

Gavin Howard
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


Re: bc

2018-03-30 Thread Gavin Howard
I do not believe that it would take more time to do a scratch implementation.

This project has taken me two months to get to a solid implementation.
However, in the course of an afternoon, I wrote a script to cut a
release for toybox, and the script combines all of the needed files
into one. I can do the same for busybox, especially since the script
could basically be reused.

I am willing to write and test the script/applet. I would just like to
know if there is interest in having me do so.

Gavin Howard

On Fri, Mar 30, 2018 at 3:10 PM, Bernhard Reutner-Fischer
<rep.dot@gmail.com> wrote:
> On 30 March 2018 17:41:08 CEST, Gavin Howard <gavin.d.how...@gmail.com> wrote:
>>Hello,
>>
>>I just realized that I made a mistake and did not send the link to the
>>source to the whole mailing list.
>>
>>https://github.com/gavinhoward/bc
>
> size(1) ?
>
> Would a busyboxification take more time than a scratch implementation? Looks 
> like from the file fragmentation alone..
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


Re: bc

2018-03-30 Thread Gavin Howard
Hello,

I just realized that I made a mistake and did not send the link to the
source to the whole mailing list.

https://github.com/gavinhoward/bc

Gavin Howard

On Tue, Mar 13, 2018 at 3:41 PM, Aaro Koskinen <aaro.koski...@iki.fi> wrote:
> Hi,
>
> On Tue, Mar 13, 2018 at 02:13:24AM +0200, Aaro Koskinen wrote:
>> On Mon, Mar 12, 2018 at 05:22:05PM -0600, Gavin Howard wrote:
>> > Over the past month or so, I have written a standalone bc. I have
>> > written it so it could be integrated into busybox/toybox, etc on an
>> > automated basis (ie, I submit a new release soon before a busybox
>> > release). Would you be interested?
>>
>> Where is the source?
>
> FWIW, as an user I would be happy to see "bc" applet in busybox, as
> it's needed to build the Linux kernel, so mandatory for any minimal
> self-hosting busybox-based Linux systems. Also GNU bc is not well these
> days, the latest release broke cross-compilation, and also added depencies
> to other GNU extensions/tools.
>
> A.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


bc

2018-03-12 Thread Gavin Howard
Hello,

Over the past month or so, I have written a standalone bc. I have
written it so it could be integrated into busybox/toybox, etc on an
automated basis (ie, I submit a new release soon before a busybox
release). Would you be interested?

Gavin Howard
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox