4 ms·
All sounds good in theory but in practice in a large enough project many of the suggestions are not practical. Recently I was looking at rsyslog's source code.
by borplk 10y ago
All sounds good in theory but in practice in a large enough project many of the suggestions are not practical.
Recently I was looking at rsyslog's source code.
Look at this "simple" file for example,
https://github.com/rsyslog/rsyslog/blob/master/plugins/omstdout/omstdout.c https://github.com/rsyslog/rsyslog/blob/master/plugins/omstd...
It's an output module for rsyslog that logs to stdout.
I wanted to gouge my eyes out.
- Sir_Cmpwn 10y agoThis code fails to follow many of my suggestions. - Liberal use of macros - Horribly unreadable coding style - Needless use of compiler extensions - Very poorly organized code This is awful code because the authors are morons, not because the language is bad. They could stand to read this blog post.
- borplk 10y agoI'd say that's a bit naive. How would you go about writing something like rsyslog without doing the things they have done? How would you create rsyslog's flexible (I'm not saying it's "good") module system where people can write their own pluggable custom input/output modules? Your comment somehow suggests that it's possible to achieve everything they have achieved in terms of functionality while doing it in a much much better way that they just don't know about. If people writing rsyslog aren't good enough to do it then 99% of other people aren't either. So let's not blame people, it's the weakness of the language.
- Sir_Cmpwn 10y agoAre you serious? You could start by fixing that godawful code style and removing the macros. You could change these variable names and function names to make more sense (CHKiRet? omsdRegCFSLineHdlr? What the hell?). The compiler extensions are unnecessary and can just be removed. None of these changes have any impact on the functionality of rsyslog.
- brogrammer2 10y agoCool down a bit, would you? You could get your point across without using that tone.
- borplk 10y agoMaybe you are not too familiar with rsyslog. Those are modules that get compiled into rsyslog itself. If you attempted to create such a module system you will very soon see that doing a lot of the things that they have done is unavoidable (weird naming and so on aside). I see this attitude where people suggest that writing beautiful secure wonderful C code is possible however just somehow all people are just too dumb to be able to do it. Every example you show them they say "nah, that person? also too dumb". You can find similar crap like that example that I pasted in all popular C projects including the Linux kernel and Redis. At some point does something become not fit for purpose? I'd say we'd make more progress if we become less tolerant of technologies that disrespect us as humans. If only an elite subset of people are good enough to program in C. I'd say that's because C is not good enough, not because everyone else is too bad for C.
- Sir_Cmpwn 10y agoHave I said anything that implies the module system is bad here? None of the guidelines in my article nor my comments here have anything to say about their module system. You could easily make this style better, name things better, and expand the macros and it would (1) work exactly the same and (2) be much better code for it.
- birc_a 10y agoI'm not sure which parts of that you are referring to as unavoidable since C can do modules just fine without any mess. Define the functions you want to implement as static functions and add something like this at the bottom of the file: outputmodule_t omstdout = { .name = "omstdout", .description = "blah", .init = prepare_stdout, .do_action = write_to_stdout } #if COMPILING_FOR_RUNTIME_LOADING outputmodule_t *rsyslog_outputmodule = &omstdout; #endif Then either stick `omstdout` in a list of modules known at compile-time or compile separately and use dlopen/dlsym to get the one necessary symbol. No GNUisms, no language extensions, no linker tricks, no macro magic. C has problems but it's not that hard to create readable code in it.
- ktRolster 10y agoI did find this line of code quite entertaining: if((r = write(1, toWrite, len)) != (int) len) { /* 1 is stdout! */ If you're going to go to the effort to add the comment, why not just use stdout instead of 1?
- pcwalton 10y ago> This is awful code because the authors are morons, not because the language is bad. Rainer Gerhards is a researcher who has published in peer-reviewed journals [1]. Can we please end this "people who write code in a style I don't prefer are idiots" meme? [1]: https://www.researchgate.net/profile/Rainer_Gerhards https://www.researchgate.net/profile/Rainer_Gerhards
- Sir_Cmpwn 10y agoIf you don't think this code is awful then we aren't have a meaningful conversation about it. Smart people can write garbage code, too, this is great evidence for it.
- makapuf 10y agoGood researcher and good coder is definitely not always the same (not specifically talking about Mr Gerhards - but this code is UURGH. Redis also has modules). Most of the research code I've seen if definitely not clean code. And needn't be. (Version Control ? what's that ?)
- wruza 10y agoWhile I read this thread I see how people make arguments and present working production solutions, but you often use 'terrible', 'horrible' and 'very' words to value them. Please don't take it as personal attack, but I think these words are purely emotional and should be thrown out of discussion. This is actually a rule for science papers in my country: you throw out all non-technical adjectives and auxiliary words and if text retains its sense, it is approved. If sentences begin to appear unfinished, then these are removed too, because they have no technical meaning. We all love simple things, but many of presented techniques evolved over decades of real non-helloworld development, and not using these leads to programming errors and bloating again. All these people are not stupid — that's not 'look mom, without hands', that's professional experience.