[insidious]

You don't need '&' before the function name in your function pointer array because they will decay to the appropriate pointer types.

But why do you free args twice here?

Code:

do {    printf("%c%s%c%c ", '$', login, '_', '>');    args = getArgs(&args_size);    status = execute(args);    free_a(args, args_size);    free(args); } while (status); free_a(args, args_size); free(args); free(login);

There's not much reason to be using the heap here either way for those allocations that stay within main. sizeof(char) is guaranteed to be 1 too, and why not use a macro if you're going to use sizeof() operator to check the size of things? More efficient than using a function that you haven't even marked as inline (sizeof is a compile-time operator that you place within a runtime function, and you can't guarantee that it'll be inlined unless compiler optimization is enabled and deals with that for you):

Code:

int num_builtins() {    return sizeof(builtin_str) / sizeof(char *); }

sizeof() returns size_t though; you could have done:

Code:

size_t num_builtins() {    return sizeof(builtin_str) / sizeof(*builtin_str); }

Or a generic macro:

Code:

#define SIZEOFA(arr) (sizeof(arr) / sizeof(*arr))

Lots of other performance improvements can be made.

Hey thanks for the feedback. It's deadweek/finals right now and I'm working on a more intensive final project so I don't have time to correct much of the things you have pointed out here, but I will surely get to it after the next week+ahalf is over.


To answer some of your questions:

But why do you free args twice here?

I don't remember exactly why I freed args twice here, I would have to revisit this code. iirc I was debugging with valgrind and was getting memory leaks, and that fixed it. To my knowledge running this program, it does not trigger any invalid frees.

I use the Heap here to avoid some possible buffer overflows in the shell. Using the heap allows me to do a variety of things, namely, dynamic allocation of arguments. So, if a user of this shell enters some ridiculously long string characters the program does not fail.
The implementation of this dynamic reallocation is in readWord/getArgs iirc


I will take a longer, harder look at this in the near future.
Thanks,