On Thu, 9 Jun 2016 19:18:21 +0200
Markus Wichmann <nullplan_AT_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 <dev_AT_frign.de>
Received on Thu Jun 09 2016 - 19:35:08 CEST