Terminal calculator made for programmers working with multiple number representations, sizes, and overall close to the bits

$ valgrind --leak-check=full ./pcalc
==28067== Memcheck, a memory error detector
==28067== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==28067== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info
==28067== Command: ./pcalc
==28067==
==28067== error calling PR_SET_PTRACER, vgdb might block
==28067==
==28067== HEAP SUMMARY:
==28067==     in use at exit: 104,380 bytes in 178 blocks
==28067==   total heap usage: 190 allocs, 12 frees, 117,482 bytes allocated
==28067==
==28067== LEAK SUMMARY:
==28067==    definitely lost: 0 bytes in 0 blocks
==28067==    indirectly lost: 0 bytes in 0 blocks
==28067==      possibly lost: 0 bytes in 0 blocks
==28067==    still reachable: 104,380 bytes in 178 blocks
==28067==         suppressed: 0 bytes in 0 blocks
==28067== Reachable blocks (those to which a pointer was found) are not shown.
==28067== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==28067==
==28067== For lists of detected and suppressed errors, rerun with: -s
==28067== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

The calculator might not be a big program but there is some memory leaks. As seen above, when starting through Valgrind, it shows, when the program has been stopped by CTRL-C that there is memory leakage. I see that in your numberstack.c you use malloc().

The amount of allocations at initialization is 190, whereas quitting instantly only causes 12 frees.

Perhaps i would recommend that you create a teardown function that tearsdown all your allocated structures. As well that this teardown functionality occurs by some character that is used in the input pane in the program.

In short;

• Give the program an ability to type q or Q in input field (Quit the calculator)
• when this action is done, teardown internal structures in order to free memory

Maybe this is not your intention with the program but its a good practice to make as low of a memory leakage as possible.

After some testing I found a solution that updates the borders and this only when the terminal got resized. I am aware of the problem that the displaywin is sometimes one terminal column off but its not that noticable.

DevManu-de

I implemented a parser that cleans up the code A LOT, and implements expressions with priorities.

I also added capabilities for testing (flag -c) and wrote a small script that runs some small tests (that we need to expand on)

This pull request replaces the confusing part of process_input and simplifies it a lot. It adds two files parser.h and parser.c that tokenize the input, compile the tokens into an expression, and resolve the expression into a value

It's a major upgrade, however it might still have some bugs - we'll deal with them as they come.

Hello everyone,

Currently pcalc only takes one operator per input line. But if you enter multiple you end up with a wrong result and a messed up History : display.

I think this is becomming more important because we have implemted the history scrolling.

DevManu-de

-h normally means "help". Also, prefer getopt_long; be nice to your users

Right now we're using getopt to handle the user options. It would be better to replace this with getopt_long and add the necessary structures to handle long options like --help

Also, right now the option -h hides the history. It would be good to come up with another letter for history, and start using h for help.

Finally, we don't have a -v, --version option. I thought about hardcoding the version in the program, but if there's a better way let me know.

Thank you, ~romes

Known bug: When mixing 0x with 0b with decimals in multiple-operators math sometimes the history is not well displayed

Can someone test this out before I merge it into master?

Thank you ~romes

A fix for #33. Its not very well done probably, and it would probably be a lot neater and would actually use the curses cursor instead of an underscore to mark where you are typing if it used curses more, but this will probably do for the time.

(Some edits made from the Reddit post)

• [x] draw.c:
  • [x] be aware that the screen can be resized
  • [x] init_gui. Avoid the long line. You're allowed to call mvwprintw more that once, you know ... Also, when you're not doing any formatting, there's mvwaddstr (though there is something to be said for uniformity).
  • [ ] printbinary: instead of numbers, I would just use the words "high" and "low" in opposite corners. Or if you keep numbers, use 63..48 instead of 64.
    • [ ] If you aggressively eliminate whitespace (maybe change color or attributes?), 00000000_00000000__00000000_00000000___00000000_00000000__00000000_00000000 is only 75 columns.
  • [x] sweepline: arguments have misleading names. Some calls might be replaceable by wclrtobot or werase.
• [x] history.c:
  • [x] add_to_history:
    • [x] resize by a multiple, not a constant, to avoid quadratic behavior
    • [x] should also not know about ncurses
    • [x] refactor out the resize logic
  • [x] add_number_to_history:
    • [x] refactor out the int-to-binary conversion to a separate function (and don't use sprintf for single chars)
    • [x] This is an odd place to call wrefresh. Move it out to the main loop.
  • [x] browsehistory:
    • [x] document mode and/or call it something else (dir, delta, ...)
    • [ ] searchHistory is currently hard-coded here (but possibly this should all go away in favor of readline's internal history)
      • [ ] to help avoid this kind of thing, simply refuse to have any global variables - make everything local to main or similar
• [x] main.c:
  • [x] never put extern in a .c file; move it to an appropriate header
    • [x] use the compiler options -Werror=missing-declarations -Werror=redundant-decls to help enforce this, and always put the corresponding header as the first include of the .c file. Use static as appropriate.
  • [x] -h normally means "help". Also, prefer getopt_long; be nice to your users
  • [x] I'm generally not happy with the whole layout of this code, so I didn't look too closely at it. There's too much duplication and mixed high-level logic with low-level logic. Now that you have something working consider taking a step back and thinking about how the program should work cleanly. I have some ideas here, but this margin is too
  • [x] don't pass magic numbers to add_number_to_history
  • [x] apply_operations: the pointer comparison *current_op != operations is different from every place that sets *current_op = &operations[0];. Honestly I would just use NULL, or if you don't like that, have a NULL_OPERATION constant. Note that there's currently no reason that the operations are in an array at all! (but that should be fixed, see below - but even so, it should not be exposed, which will make it easy to update)
  • [ ] get_input: since you're using curses already, use it. It won't have all the bugs that your code has.
• [x] numberstack.c:
  • [x] create_numberstack also needs error-checking
  • [x] refactor resize_numberstack so it doesn't need to know about curses. This could involve either returning NULL so the caller can deal with it, or else relying on a previously-installed atexit or similar hook.
• [x] operators.c:
  • [x] getopcode should be about 4 lines long - the operators are in the table!
  • [ ] I suggest implementing multi-character opcodes sooner rather than later.
  • [x] globalmask logic should probably be refactored out of every function, to do it just once in apply_operations
  • [x] twos_complement can be just -a.
  • [x] sr can be simplified if you use unsigned math (which you should, because this is UB)
  • [x] rr and rl also have UB when shift is 0.
  • [x] also, more usual names would be ror and shr
  • [ ] division and modulus:
    • [ ] keep these together
    • [ ] you need 3 versions of these: unsigned, signed following numerator, signed following denominator (different platforms historically chose different signed implementations, but later C standardized what x86 does - Python notably does the opposite)
    • [ ] division and modulus checking needs to handle the error of globalmask / -1 for the signed versions
  • [x] output should probably be some struct value_and_flags, which more-cleanly handles SIGFPE but also ZF SF OF PF CF AF. Use GCC's overflow intrinsics (and other intrinsics) for some of those. Actually, possibly mask should be included as well ... this would involve masking before the operations as well (easy enough in apply_operations), and removing the whole "mask the whole history" logic.

Hello, I managed to implement following things.

• No duplicates that come after one another
• Infinite history size
• Scrolling through the stack from the top

Can you guys please test it for bugs.

Thanks.

DevManu-de

Hello,

Im working on the history scroll feature and I have a new history system implemented (without scrolling just storing the data) but Im not very happy with it maybe I will start over again. I have problems following the code in main.c line 198 to 234. Maybe someone could provide more documentation.

This is the code from line 198 to 234.

    if (op != NULL) {
        char opchar[2];
        opchar[0] = *op;
        opchar[1] = '\0';
        *current_op = getopcode(*op);
        char * token = strtok(in, opchar);
        int operationInStack = 0;
        if(token == NULL) {
            if(strcmp(opchar, history.records[history.size-1]))
                add_to_history(opchar);
        } else if (token != NULL && token < op) {
            clear_numberstack(numbers);
            clear_history();
        } else if (token != NULL && op < token) {   
            if(strcmp(opchar, history.records[history.size-1]))
                add_to_history(opchar);
            operationInStack = 1;
        }
        while (token != NULL) {

            long long aux = pushnumber(token, numbers);
            
            if(strstr(token, "0b") != NULL)
                add_number_to_history(aux, 2);
            else if (strstr(token, "0x") != NULL)
                add_number_to_history(aux, 1);
            else
                add_number_to_history(aux, 0);

            if(!operationInStack && strcmp(opchar, history.records[history.size-1])) {
                add_to_history(opchar);
                operationInStack = 1;
            }
        	token = strtok(NULL, opchar);
        }
    }

In addition to that, can you provide a little more documentation on what exactly numberstack holds and how it is used.

Furthermore I want to ask if there is a problem if I split the main.c in smaller files this will be handy when the project grows and also for the maintainer and contributors now.

If you want to take a look at the current state (https://github.com/DevManu-de/programmer-calculator/tree/history-scroll). Please dont hesitate to give feedback.

Thanks in advance for the answer. DevManu-de

Your program is really fast, but it doesn't calculate decimal numbers and negative numbers. I know it needs a lot of work to be done, but it should be completed as a terminal application with an update like this.

Hello.

Currently, the change number of bits command seems to break things, as it will consume every input that has the substring "cb" anywhere within it. This includes all hex numbers that match this, so numbers like 0xcb are impossible to input as regular numbers, though upper case does work.

The problem is most likely here: https://github.com/alt-romes/programmer-calculator/blob/060c9969170e78e9259e801b3bdfa548fdbbb504/src/main.c#L206

As an easy fix, the substring could be changed to something that doesn't clash with anything else, like "bits".

Best regards.