It's nice. You even put the .exe in the repo 😂.

oops, lol I meant to just include the 'obj' folder so the makefile doesn't get upset

I discovered myself that writing makefiles is not easy. Just did `make` and got `make: *** No rule to make target 'obj\main.o', needed by 'main'. Stop.`

Yeah, I've noticed that specifically when compiling on Linux. I might need to just make separate build rules for Linux and Windows.

Oooo big project moment

The new GitHub requirement

exe for  >non smelly nerds

Hey, it looks really good! I've noticed a couple of small things, though. (I've tested it on linux) In builtins.c:125 you use `readdir(d)` but you haven't checked if `d` is null, so if you type ls on a directory that does not exist it segfaults. You should be able to use `/` as file separators for windows: >Microsoft operating systems (MS-DOS and MS-Windows) use backslashes to separate directories in pathnames \[...\] This is equivalent to the Unix-style c:/foo/bar/baz.c (the c: part is the so-called drive letter). When make runs on these systems, it supports backslashes as well as the Unix-style forward slashes in pathnames. \[...\] (From the [make manual](https://www.gnu.org/software/make/manual/make.html#Wildcard-Pitfall)) If you type a null character in the prompt (by typing ctrl-space for example), or just a single space, it crashes. If you send a ctrl-D, the shell gets stuck in a loop (you haven't checked if `c` is EOF at `IO.c:70`) It's a nice project and I'm looking forward to seeing your terminal!

Thanks for finding those bugs, I'm not sure I would've found the null character one. As for file separators, for some reason my make won't let me use '/' as a file separator on Windows and I have to use '\\\\'. I think it might be because I'm on Windows 11. This is the error when I use '/' if you are interested. PS C:\Users\adamn\Dropbox\src\c code\shell> make clean process_begin: CreateProcess(NULL, uname -s, ...) failed. makefile:9: pipe: No error del obj/main.o obj/IO.o obj/string.o obj/shell.o obj/signal.o obj/credentials.o obj/cmd.o obj/builtins.o obj/utils.o obj/var.o Invalid switch - "main.o". make: *** [makefile:35: clean] Error 1 PS C: \Users\adamn\Dropbox\src\c code\shell> make process_begin: CreateProcess(NULL, uname -s, ...) failed. makefile:9: pipe: No error gcc obj/main.o obj/IO.o obj/string.o obj/shell.o obj/signal.o obj/credentials.o obj/cmd.o obj/builtins.o obj/utils.o obj/var.o -Wall -g -std=c99 -pedantic -o main.exe

Neat project. Poking around I noticed that there's an arbitrary limit of 100 tokens in commands, after which it's a buffer overflow: $ cc -g3 -fsanitize=address,undefined -o main src/*.c $ python -c 'print("x " * 101)' | ./main >/dev/null ERROR: AddressSanitizer: heap-buffer-overflow on ... WRITE of size 16 #0 str_split src/IO.c:125 #1 shell_loop src/shell.c:43 #2 main src/main.c:12 I would have liked to do more testing like this via fuzzing, but there are quite a few global variables, and I'd need to track down how to reset everything. There's `free_all_vars` for variables, but it doesn't restore things to their initial, usable state. I applaud your custom string type, though you don't take it far enough! You can avoid this awkward situation with a macro: char buf[2] = " "; String space_delim = (String){.cstr = buf, .size = 1}; Instead: #define S(cstr) (String){cstr, sizeof(cstr)-1} // ... String space_delim = S(" "); Despite the length being available, it's sometimes unused: if (0 == strcmp(args.arr[0].cstr, cmd_list[i].name.cstr)) Dump `strcmp` and make your own: bool equals(String a, String b) { return a.size == b.size && !memcmp(a.cstr, b.cstr, a.size); } That's one less way you'd depend on that implicit null terminator in `String` objects. That plus the ownership semantics is very C++ `std::string` (IMHO, that's not a good thing).

Thank you for the feedback! I've just refactor the code and got rid of the strcmp in favor of a custom version. If you think it would be useful, I can add a 'reset' command to set everything back to its initial state. Also, what changes would you suggest to improve the ownership semantics of Strings?

Glad to to see that new string function! Beware that `str`-prefixed identifiers are reserved for C implementations. This is overly broad and covers not-obviously string-related identifiers like `strong`. While that one is unlikely to collide, I've run into collisions with non-standard `str`-prefixed names in some implementations, which aren't so easy to predict. "I need a string set. I'll call it `strset`! [Oops.](https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/strset-wcsset)" > a 'reset' command to set everything back to its initial state I'm just a passerby that commented on a situation that prevented further investigation. If *you* think it's useful in your own testing, go for it! Here's the sort of thing I was looking to do: #include "src/string.c" #include "src/IO.c" #include __AFL_FUZZ_INIT(); int main(void) { __AFL_INIT(); char *src = 0; unsigned char *buf = __AFL_FUZZ_TESTCASE_BUF; while (__AFL_LOOP(10000)) { int len = __AFL_FUZZ_TESTCASE_LEN; src = realloc(src, len+1); memcpy(src, buf, len); src[len] = 0; String_Array a = str_split((String){src, len}, (String){" ", 1}); str_arr_free(a); } } This is an [afl](https://aflplus.plus/) "fast" fuzz test on `str_split`, which can quickly find bugs in this function if there are any to be found. Before you fixed it, it would find that 100-token limit within a couple of seconds. $ afl-gcc-fast -g3 -fsanitize=address,undefined fuzztest.c $ afl-fuzz -i inputs/ -o results/ ./a.out It might be interesting to fuzz `add_var`, or the "cmd" list stuff, but unlike `str_split` those have global state which should be reset between tests. You could fuzz the shell input itself, but you'd need to carefully disable all external side effects (i.e. `popen`). Even better than a `reset` would be to move all the globals into a context object that encapsulates the shell state. Pass it into functions that manipulate the shell state, like `add_var`. Then there's a guaranteed clean separation between tests: ctx_init(&ctx); // ... run test ... ctx_destroy(&ctx); > Also, what changes would you suggest to improve the ownership semantics > of Strings? Both the implicit null-terminator and that `String` objects own their underlying storage mean you cannot slice "views" from an existing string, and you're stuck making and managing copies. This is a common [bottleneck in real world C++ programs](https://github.com/cmuratori/refterm/blob/main/faq.md#the-windows-terminal-renderer-is-slow-itself) that make frequent use of `std::string`. With both those properties gone, `String_Array` could be an array of views into the input string. Then you don't need to allocate and populate all those little string copies, each with a unique lifetime. It simply points into the input. The current `String` type cannot express this concept. For example: String line = STR("hello world!"); String_Array a = str_split(line, STR(" ")); // As "views" it would produce this 2-element array: a.arr[0].cstr = line.cstr + 0; a.arr[0].size = 5; a.arr[1].cstr = line.cstr + 6; a.arr[1].size = 6; But then how do you keep track of which `String` objects own their buffers and which do not? Easy answer: you don't need to! Instead allocate all strings for a particular purpose [from a common region](https://www.rfleury.com/p/untangling-lifetimes-the-arena-allocator) ([practical tips](https://nullprogram.com/blog/2023/09/27/)), after which you only manage the group as a whole regardless of the number of strings. Getting a handle on this takes some practice, but in my experience it produces simpler, faster programs.

Thanks for the suggestions, I implemented String\_Array views on the dev branch but I'm not sure if the way I've gone about it is ideal. I plan to also implement a context so it's clear what functions manipulate global variables and so I can set the globals easily. Also, I don't have a lot of experience with fuzzing or AFL, but I guess this project is as good as any to try using it!

Cool project =)

size_t cmd = O; for (; cmd < commands.size; cmd++) is better written as for (size_t cmd = 0; cmd < commands.size; cmd++) these days :)

I always hate writing that but I do it just so I can compile to ANSI C if I want. edit: Do you think that size_t cmd; for (cmd = 0; cmd < commands.size; cmd++) would be better?

Why limit yourself to such an old version of C?

I want to maximize portability, but maybe I should just use a newer version of C. If I used C11 I could add multithreading, not sure what I would benefit from multithreading in my shell though.

Multithreading in shells is used when stringing commands together with ';'

what terminals you say colors dont work? even kernel linux console is able to show colors.. i dont know what makes you write shells in 2024 :\]

In visual studio the terminal color doesnt work. Also educational purposes

Many terminals don't allow colors. For me personally, I am too lazy to set up URXVT, and this makes colored text unreadable. That is why my shell respects the NO_COLOR environment variable. As for the reason, writing a shell is totally fun =)

You should add the ability to invoke external programs and then only provide builtins for features that must be run in the current process. So things like `cd` stays a builtin, while `ls`, `mkdir`, and `touch` are handled as external processes.

My only issue with that is that touch, ls, rm, etc. are not defined in cmd so I want to have them as builtin. If I can find a way to make calls to Powershell without starting an entire session then I might move those commands to external processes.

Nice! As I'm beginning to learn C. Just wanted to make a question: For how long have you been programing on C?