rank04 failing

[CarloCattano]

I'm consistently failing microshell and Im not sure why.
Could it be an issue with the tester on cluster computers or am I missing something ?
Thanks for any help, I thought I was ready for the exam :-(

microshell.txt
trace.txt

[JCluzet]

Here's what the trace shows: bin/ls: cannot access 'microshell.c': No such file or directory can you go to your rendu/ folder and run the tree command and send me the result? I think your files are not well located.

[CarloCattano]

pwd

/home/carlo/42/study/42_EXAM/rendu

tree

└── microshell
    └── microshell.c

[CarloCattano]

there was a mistake on the main, av should be av += i
However it still FAILS and now it even crashed the 42-EXAM

/usr/src/debug/gcc/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/basic_string.h:1329: std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::reference std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::back() [with _CharT = char; _Traits = std::char_traits<char>; _Alloc = std::allocator<char>; reference = char&]: Assertion '!empty()' failed.
.system/launch.sh: line 264: 28153 Aborted                 (core dumped) ./.system/a.out
make: *** [Makefile:14: all] Error 134

[JCluzet]

After a quick check, I see that there was recently a PullRequest made by @hcivici on examRank04.

@hcivici do you know if the error could be yours?
Thanks :)

[CarloCattano]

can confirm that in commit a6233ca it passes

[JCluzet]

I'm going to let @hcivici fix this problem since he's the one who made the pull request, if there's no response from him I'll take care of it on my end.
Thank you for reporting this 👍

for now i revert the @hcivici pull request

[hcivici]

Clarifications

@CarloCattano Your code fails here as seen in the trace.txt by #test1 1st and 2nd lines not being in the correct order because you are executing one command at a time. You should not use waitpid() right after every fork(). I stated some information on Fixes section for microshell.c:

By executing one command at a time:
-> All the commands in the same pipeline that should have started at the same time would wait one before themself to finish executing
-> Code would rely on pipe buffer capacity

You can try these commands to see it yourself:

     [shell]: sleep 1 | echo hello; sleep 1 | echo bye 
[microshell]: /bin/sleep 1 "|" /bin/echo hello ";" /bin/sleep 1 "|" /bin/echo bye

Also "cat | cat | ls should behave the normal way" -from minishell evo page. If you can make this command work as expected, you most likely won't have an fd leak. (Although you can still have duplicates of the same end of the pipe on a process which shouldn't be a problem)

Your code has other problematic behaviours such as:

• Modifying the current shell's STDIN at line 41 using dup2()
• Incorrect arg reading and execution after changing av += 1 to av += i (You probably have re-written the whole algorithm after sending it here)

I haven't touched any ls commands. As shown in the tree, microshell.c isn't in the rendu folder.

While examining the trace, I came across 65537 nulls in the file. I misread the (not very descriptive) documentation of tee command and wasn't aware that it also wrote into STDOUT along with the file(s) provided. Although it didn't (and wouldn't) cause issues (other than trace file being 64kb+1 larger :D) since a properly working rendu code would produce the same output as the internal one.

And about the crash (abort) situation, it boils down to: string, back() and Assertion '!empty()' failed. back() function returns the last character in a string. In case if you wonder what happens on an empty string, you can take a look at the implementation of back() in my machine below.

It tries to read str[-1] when string is empty
After searching for back(), it turns out the only use for it is to trim trailing spaces from the line. Unrelated to ExamRank04.

I managed to reproduce the same crash with the following steps

• Add -D_GLIBCXX_DEBUG option to g++ on launch.sh and make
• After starting the exam, just press enter.

Still not sure how you compiled the project on debug mode.

Conclusions

• As far as I examined the issue, I haven't came across a problem related to my pull request other than tee command making trace file bigger by printing 64kb of nulls. This isn't a problem if the rendu code is correct.
• Provided code has some problems on the main loop and doesn't actually mimic how execution part is handled in the shell.
• Pressing enter with an empty line causes problems which is not something about ExamRank04.

Solutions

• Although it is crucial to execute all the commands in the same pipeline at the same time and as a byproduct not rely on pipe buffer capacity, if you guys have insights on whether or not Moulinette tests such a thing and make people fail in the real exam, we can decide to either keep or remove the first waitpid() test and the test_full_pipe_buffer from the test.sh
• When the line is empty, either skip the line or use other functions for trimming.

[CarloCattano]

1- microshell.c is/was in the rendu folder
2- (You probably have re-written the whole algorithm after sending it here)
Simply no , I had a mistake that I pointed out and fixed it as I said.

I understand most of what you are saying , and while yes, your pull request might be correct in all senses( apart from that crash that occurred once to me) , but frankly speaking , do you want to make the tester stricter than the exam ?

I just passed today with that solution ( not mine ) as seen here https://github.com/pasqualerossi/42-School-Exam-Rank-04/blob/main/microshell.c

should behave the normal way" -from minishell evo page.
we are talking about microshell right ? I dont recall seeing such a requirement in the micro shell subject

[hcivici]

@CarloCattano

microshell.c is/was in the rendu folder

I mean it is not right in the rendu/ folder, it is right inside rendu/microshell/ which is the correct location. The ls test on the test.sh runs on rendu/ folder so there is no microshell.c there.

I had a mistake that I pointed out and fixed it as I said

Yes, I probably ran the old compiled version which had av += 1. Sorry about that.

do you want to make the tester stricter than the exam ?
I just passed today with that solution

Not really. If Moulinette doesn't test such a thing, it becomes redundant for us to do so. Thanks for letting us know. I am going to remove the related tests from my pull request #153 (waitpid right after every fork and test_full_pipe_buffer_capacity)

I dont recall seeing such a requirement in the micro shell subject

Well, I have added fd leak tests just in case because there is a related "hint" in the subject.en.txt as follows:

Hints:
Do not leak file descriptors!

Maybe this refers to leaking pipe fds in the parent process so that we would reach our max open fd limit (30) and not be able to create new pipes for the child processes.

• Your program should be able to manage more than hundreds of "|" even if we limit the number of "open files" to less than 30.

This is already adjusted by ulimit -n 30 at the top of the test.sh
Now you may ask why do I refer to minishell: If you were to run all commands in the same pipeline at the same time, cat | cat | ls would exit after you send 2 lines of input from the terminal. When ls exits, reading end of the 2nd pipe is closed, if 2nd cat tries to write() into that same pipe's write end while the read end is closed, it gets SIGPIPE signal and terminates. Same event happens for the first pipe because now 2nd cat is exited and 1st pipe's read end is closed...
If you were to have an fd leak, either writing processes wouldn't get SIGPIPE or reading processes would wait for all write ends to close so that they know they can't get data from that pipe anymore when read() should have returned 0 for them. This model isn't reproduced and tested since Moulinette allows us to run 1 command at a time as you were able to pass the exam. Conclusion: FD leak only applies to parent process reaching its 30 fd limit by forgetting to close pipes 🤷 So, I am also going to remove test_fd_leak 🏳️