A little sloppy with the error handling, but pretty neat.
E.g. log_append() doesn't check fopen return value, malloc() return isn't checked, and a write() can return a partial write, thus needs a loop. fcntl().
Also write() should check for EINTR&EAGAIN.
And also there's no handling for nonblocking write.
If the user types too much while the network glitches it seems that this could cause the client to exit().
Probably the correct way is to not read from stdin unless poll() returns that writing to the socket is safe, and vice versa.
connect() errors should print where they failed to connect.
And:
$ ./kirc
Nick not specified: Success
And in my first test I got:
Write to socket: Resource temporarily unavailable
And I see a lot of NULL pointers being printed when I connect to e.g. freenode.
And so on, and so on…
For these things I recommend re-reading the manpage for every libc and syscall you call, and check how they can fail, and consider how you can handle that failure.
307 lines of pure C is a pretty neat minimal actually usable client, so I'm not saying it's not well done. But that thing about "… and do it well" (from the readme) means doing all of the above.
The problem, of course, is that fixing these problems well is "the other 90% of the work", especially when coding in C.
And this is the main reason I avoid C when I can. You can't just call "write()". You have to write a 5-10 line wrapper function, and then all callers need a few lines of error handling. And so it goes for everything, until your program is no longer the nice 300 line neat thing you can almost write from memory.
So it's a good start. But it's pretty fragile in its current form.
i appreciate the honest feedback! definitely a work in progress and i have learned a lot since i started ~1mo ago on this. will take your suggestions and add them to my ever growing “todo” list. ;)
small digression, but I thought that at least on Linux, malloc() never returns an error because the actual allocation happens lazily, when the memory is first used?
Cases where memory allocations fail on linux include: hitting memory resource limits (ulimit), overcommit behaviour is set to stricter than the default via sysctl, kernel heuristics with the default overcommit settings end up failing allocation, container / cgroups limits, 32-bit virtual address space is exhausted - i'm sure there are more.
The default overcommit-within-reason algorithm is designed to deny allocations that are obviously unrealistic I think.
Using semi conservative ulimit settings is pretty common in interactive use to catch runaway / swapped to death situations.
Take a look at vm.max_map_count. A lot of systems have that set artificially low and that will force a memory allocator to fail and return NULL. I can confirm this is the case for jemalloc.
Also, on Linux, only 48 of 64 bits are available for VM addressing. That leaves you with about 250TB of address space. Seems like a lot until you start using VM for disk mmaps. I have actually seen this limit hit.
Also it's 48-bit just on current amd64 chips, it's not an architectural amd64 property - your binary is compatible with >48bit VA space of the future (unless you yourself misguidedly bake the 48-bit assumption into it). But I guess the point was to point out lower bounds.
It's sloppy coding and not portable. If you really don't want to check for malloc() return value because you consider that it shouldn't fail (or that you won't be able to recover from it anyway) just implement an xmalloc() that just aborts on failure and do that. At least it'll crash "cleanly" this way.
On 64 bits malloc basically always succeeds, because you are very likely not running out of virtual memory. You can totally go ahead and malloc terabytes of RAM.
On 32 bits malloc is going to fail once your virtual memory space is full (~3 GB).
Error checking is somewhat redundant for most applications, since you're going to abort anyway, and if there are no pages available on access - well you're getting SIGSEGV anyway, just like you would accessing a null pointer (except on embedded devices). But beware of the exceptions. In C especially it's common to use null pointers as a flag... one of the reasons why new/delete in C++ is preferable; it throws and thus aborts when it fails, which you don't care to handle, so that's about perfect for a C++ exception.
In addition to what others have said, which basically boils down to "it'll never return an error… unless it does", there's also the aspect where if you start assuming things about the kernel environment you're not coding in "POSIX C99" like the title says, but to "Debian GNU/Linux circa 2020".
And that's part of the "and do it well". A thousand years from now, if the C99&POSIX spec survives, will support correctly written code.
Dereferencing a null pointer can also be a security issue (mostly for kernels, though) so one should assume a malicious general environment.
E.g. log_append() doesn't check fopen return value, malloc() return isn't checked, and a write() can return a partial write, thus needs a loop. fcntl().
Also write() should check for EINTR&EAGAIN.
And also there's no handling for nonblocking write.
If the user types too much while the network glitches it seems that this could cause the client to exit().
Probably the correct way is to not read from stdin unless poll() returns that writing to the socket is safe, and vice versa.
connect() errors should print where they failed to connect.
And: $ ./kirc Nick not specified: Success
And in my first test I got: Write to socket: Resource temporarily unavailable
And I see a lot of NULL pointers being printed when I connect to e.g. freenode.
And so on, and so on…
For these things I recommend re-reading the manpage for every libc and syscall you call, and check how they can fail, and consider how you can handle that failure.
307 lines of pure C is a pretty neat minimal actually usable client, so I'm not saying it's not well done. But that thing about "… and do it well" (from the readme) means doing all of the above.
The problem, of course, is that fixing these problems well is "the other 90% of the work", especially when coding in C.
And this is the main reason I avoid C when I can. You can't just call "write()". You have to write a 5-10 line wrapper function, and then all callers need a few lines of error handling. And so it goes for everything, until your program is no longer the nice 300 line neat thing you can almost write from memory.
So it's a good start. But it's pretty fragile in its current form.