On Thu, 9 Jun 2016 19:18:21 +0200 Markus Wichmann <nullp...@gmx.net> wrote:
Hey Markus, > Dear Lord, it's been a while since I've seen such nice code make so > bafflingly bad design choices. Where to start? the suckless mailing list is not a place for religious cults. > 1. The whole ulinux thing smacks a bit of NIH syndrome to me. And of > course, it kisses portability goodbye, but then that wasn't your goal at > all. With only i386 and AMD64 supported, I wonder why this isn't in > assembler. Holy shit, I can't even put in words how much NiH it is. Compared to this, Ubuntu's "Mir" was a work of genius! > 2. You immediately block all signals on entry. That means: > - SIGINT, SIGQUIT, SIGTSTP, SIGTTOU, SIGTTIN: There is no way to > control the process from a terminal. You have to get another > terminal out to kill it. > - SIGSEGV, SIGILL, SIGBUS: Undefined behaviour if your code does > anything to warrant the sending of those signals. > - SIGCHLD: Zombies and no way of knowing about them. Why don't you > just ignore it? > - SIGXCPU, SIGXFSZ: No way to gracefully close on reaching ulimits. > And so on, and so forth. And for what? So you can handle signals using > signalfd. That would be good if you actually did that. However, that > only happens if the client never stalls out on you. Yeah, the SIGCHLD thing is definitely a point to consider. If you ignore it, the program will reap children automatically. > 4. Synchronous communication, no forking, no threading --> One client at > a time. So you're using epoll on the same two sockets all the time, > which means you might as well not have bothered. I prefer poll over threads/forks, but yeah, it really is crazy here. > Still, it could easily be salvaged: Drop all the setup code for the > server socket and make the code read from stdin and write to stdout. > Then this server can be run from inetd, or through any ucspi > implementation. That will also remove the glaring issue that the program > must be run as root and doesn't drop privileges even when it's done > doing privileged things. Very good point! Definitely a very good point. It would also simplify the code a lot. > 5. Exiting at the drop of a hat: Your only error handling is to exit, > unless it was an expected error (usually EINTR). That's the opposite of > PHP, I guess, but that doesn't make it better. This is okay for one-shot-tools, but definitely not for servers. > 6. You do know that if you used a C library, you'd have access to > strchr(), strcpy(), and the like, right? Because you seem to be having a > jolly good time rewriting those over and over again. Which is a big shame. He could've saved some LOCs using a bloody libc. > 7. What's with the S_IFMT constants? You're binding hard to the syscall > interface, you can use 0666, it's not getting any less portable now. > 8. You do know that C has more loop contructs than just the endless one, > right? Because I'm seeing a lot of endless loops that are basically > do-while loops, or counting loops... C has structured programming. Use > it! Please... > 9. Oh wait, I see that you have strcpy(), you just don't use it. > Alrighty then. > And that was just what I saw in lnanosmtp.c. And I didn't check the > protocol. It's just a big fucking mess there is no need for. Sylvain, sit down again, use a fucking libc so fucking BSD users and other arch users can fucking use your shit. Then we can talk. Cheers FRIGN -- FRIGN <d...@frign.de>