Journal phantomfive's Journal: systemd 10 - code structure (a random-seed example) 18

Journal by phantomfive on Tuesday September 01, 2015 @02:25PM As an intermezzo while investigating the stability of APIs (since that's not a fun thing to do), here is an analysis of some code: examining random-seed.c, pulled September 1st, 2015 ( link here, for as long as github is around). When you come across a piece of code, the first thing you ask is, "what does this do?" In this piece of code, you can see the main() on line 34, so we know it's a stand-alone utility. That is the only function in a 165 line file, so come on Poettering, break your code into easy to read functions. It's not 1985 anymore. Since it's not clear what the utility does from the comments, a web search might be helpful, which finds the man page, which doesn't tell you much. For example, on line 44 of random-seed.c, we can see that a command-line argument is required. However, the man page doesn't tell you what options are available, it's biggest use is telling you where the seed is stored on the filesystem (if you're wondering where that file location is defined, it's in the Makefile.am). The documentation is bad. The program itself isn't helpful either, it doesn't respond to the --help option. If you give it any option that is not correct, it responds "Unknown verb '%s'." So keep guessing random verbs until you get the correct option. Moving along to my main complaint, you have to read many lines of this file before you understand what it does. A program like this can be mostly self-documenting, if you structure main correctly. Here is an example of how it could be. int main(int argc, char **argv) { init(); parseCommandLineArgs(); if(shouldLoad) { writeSeedFromFileToDevUrandom(); } else if(shouldSave) { writeSeedFromDevToFile(); } return 0; } This can still be improved. There can be comments explaining why things happen. It is much easier to see what the program does when it's broken up like this. The main algorithm should be clear. One more serious criticism about this file, hard-coding the output file in Makefile.am is double bad: first because it shouldn't be hardcoded, it should be a command-line option. Second because build files should store as little information as possible. That's a lot of criticism, but they're relatively minor. With a few fixes, the code is cleaned up. The big problems in systemd, the things that really worry me, were mentioned in journal 9. Those are the things that will cause serious degradation over time, and problems for all of us. This discussion has been archived. No new comments can be posted.

systemd 10 - code structure (a random-seed example) More Login

systemd 10 - code structure (a random-seed example)

Load All Comments Full Abbreviated Hidden /Sea Score: 5 4 3 2 1 0 -1 More

Nickname:

Password:

Public Terminal

Forgot your password? Close Close Search 18Comments Log In/Create an Account

Comments Filter: ● All ● Insightful ● Informative ● Interesting ● Funny

The Fine Print: The following comments are owned by whoever posted them. We are not responsible for them in any way.

Push it upstream? (Score:1)

byKGIII ( 973947 ) Are you going to push your changes upstream to see if they patch it? Maybe comment it and give 'em a hint?

Re: (Score:2)

byphantomfive ( 622387 ) Hmmmm......I've never met anyone who wrote super-long functions who didn't claim vehemently that is was a superior way of writing code. I wouldn't expect them to accept my changes, but maybe they would. Anyway, it would mean I have to actually test the changes, which is so much work for a lazy person like me!

Re: (Score:1)

byKGIII ( 973947 ) You do have a point. I'm really not sure what to think of SystemD. It has not negatively impacted me... Yet... It seems like it was adopted quickly - I'd heard about it one day and the next it was in the majority of distros. Well, not quite like that but you get the general idea. I am kind of interested in what the response would be if you pushed it upstream. I'd expect it to be declined but how and with what message might be telling.

Re: (Score:2)

byphantomfive ( 622387 ) Eh, if I write enough of these code reviews and nothing gets done, maybe I'll stop being lazy and start doing patches. I'd like to have a deeper understanding of it before I start doing that. I'll either start doing patches, or write a replacement ;)

Re: (Score:2)

byphantomfive ( 622387 ) Also, this stuff is relatively minor. The biggest problems were mentioned in journal 9, that is the stuff that really needs to change in systemd, and that actually worries me.

Re: (Score:2)

bycaseih ( 160668 ) I read your Journal 9 but what is all this "stuff that really needs to change in systemd?" Your post mostly talks about ethernet and unix. You also talk about "interfaces" without defining what you mean and how it relates to systemd. I was hoping to see some actual systemd code analysis, but there was none. What exactly are these things that must change in systemd? How about a specific example and a proposed solution?

Re: (Score:2)

byphantomfive ( 622387 ) I was hoping to see some actual systemd code analysis, but there was none. Journal entry 10 digs into the actual code. You also talk about "interfaces" without defining what you mean and how it relates to systemd. If you don't understand interfaces, you missed the main point of the post. That was the reason ethernet and unix were brought up. I wrote plenty of paragraphs there to try to explain it.......if you didn't understand, I don't know what I can say. What parts of what I wrote are giving you trouble?

Re: (Score:2)

byookaze ( 227977 ) You also talk about "interfaces" without defining what you mean and how it relates to systemd. If you don't understand interfaces, you missed the main point of the post. That was the reason ethernet and unix were brought up. I wrote plenty of paragraphs there to try to explain it.......if you didn't understand, I don't know what I can say. What parts of what I wrote are giving you trouble? I think his problem is the same as mine: you have not given any meaningful example of what you are talking about (bad interfaces) specific to systemd. This journal addresses a very minor issue that has nothing to do with interfaces, but with code readability. What's even stranger is that reading the code quickly, it's immediately obvious to me what the options for the commands are, and I'm no professional programmer. Also, from the man page, which is actually for the service unit, we see that this tool is no

Re: (Score:2)

byphantomfive ( 622387 ) What's even stranger is that reading the code quickly, it's immediately obvious to me what the options for the commands are If you have to read code to understand what the options are, then that's a fail from a programming perspective. You prefer involving the stack for a trivial program like that, systemd developers prefer not too. The stack is already involved quite a bit in the program the way they've written it. Don't you think that the way I rewrote it makes it easier to understand? I think his problem is the same as mine: you have not given any meaningful example of what you are talking about (bad interfaces) specific to systemd Once again, if you don't understand why interfaces are important, I'm not sure what I can do to help you. What don't you understand? By the time you finish reading that post, it should be obvious that the GUI shouldn't depend on the init syst

Re: (Score:2)

bytibit ( 1762298 ) Sometimes super-long-functions grow organically. This thing is probably so low on anyone's priority list that your changes have a good chance of being accepted. I fully agree with your sentiment. A lot of OSS code is in need of serious TLC to make it maintainable.

options? (Score:1)

byfustakrakich ( 1673220 ) "load" and "save", right? Everything else seems to be unknown.

Re: (Score:1)

byfustakrakich ( 1673220 ) I'm sorry: lines 84 or 119, else 134

Re: (Score:2)

byphantomfive ( 622387 ) Yeah, that's what I understood from reading the code, too.

Man, thank you for the reviews (Score:2)

bySaija ( 1114681 ) Thank you for the reviews about SystemD. I don't know much about init systems, as long as they let me log in on my system that's much I need to know now(I'm myself lazy). But something that worry me about SystemD it's his premature adoption by the major players on the distro game. Everyone one of them has just adopted making the lives of users like me a little complicated. Just this past weekend I was tinkering trying to mount an Android development environment on some virtualBox virtual machine. I have

Re: (Score:2)

byphantomfive ( 622387 ) I'm glad someone is reading it. If you learn from it, that will make me even happier.

Re: (Score:1)

bydbreeze ( 228599 ) Keep up the good work phantom. I'll be following along. I'm a month into my first system administrator class and hope to quiz the professor on some of this systemd issue.

Re: (Score:2)

byphantomfive ( 622387 ) Don't be disrespectful to your professors, but if you do ask him about systemd, let me know how he responds. I would be interested in hearing.

Re: (Score:2)

byookaze ( 227977 ) Thank you for the reviews about SystemD. I don't know much about init systems, as long as they let me log in on my system that's much I need to know now(I'm myself lazy). But something that worry me about SystemD it's his premature adoption by the major players on the distro game. Everyone one of them has just adopted making the lives of users like me a little complicated. The irony is that, like most people with your level of admin skills, you admit having none, but yet you want to patronize people that are way more skilled than you. Lives of users like you are not more complicated by systemd. The contradiction related to sysvinit vs systemd is obvious to every logical people: you say sysvinit didn't need tinkering because it didn't have problems, and at the same time you complain that you can't tinker with systemd like you were with sysvinit when you had a problem with it. T There may be more comments in this discussion. Without JavaScript enabled, you might want to turn on Classic Discussion System in your preferences instead.

Slashdot