4 ms·

By linking to those examples you confirmed the point you were trying to refute. From my personal perspective this is mediocre code at best. It’s written in an

By linking to those examples you confirmed the point you were trying to refute. From my personal perspective this is mediocre code at best. It’s written in an unsafe language. Littered with macros, which in C are not hygienic and are land mines in waiting. Constants defined with lowercase and not actually marked as const. Double underscores everywhere, which are the bad alternative to namespaces seen in weak languages like C. WTF does “BITBIT_NR(nr)” do!? Does it “bit” the “bits”? What is NR? Is it a “number”? Then why not use “n” and drop it from the macro name? Abbreviations everywhere: ofdr, nfdt, fds, fs, etc… Then there is a long-winded explanation of how they pack bits into an array of longs. Okay, why not make this a reusable module of code? Because C is a weak language, or because the Linux kernel is spaghetti with a dozen implementations of bit maps? \* * Note that this can drive nr *below* what we had passed if sysctl_nr_open * had been set lower between the check in expand_files() and here. Deal * with that in caller, it's cheaper that way. Huuuurk. Sorry… did I just see a data race just casually commented as “okay because it is faster if it’s horrifically unsafe? Yes? No? Maybe? Do you even know?

cookiengineer 3y ago
I think I just fell in love with you. Finally a sane person <3 Honestly, I never understood why so many developers abbreviate function names or variables. Is it so much pain to type abc<tab> for auto completion? Why not simply name fd file_descriptor? Short names are a mental barrier and a level of abstraction that can easily be avoided.
- sudosysgen 3y ago
  In some languages, that leads to terribly long lines that you then have to break down, add indentation, and it just becomes harder to read. There is happy middle.
- nemetroid 3y ago
  Anyone who has reason to read this file will have "fd" as a word in their vocabulary.
- buzer 3y ago
  Short names actually help with readability when they are commonly used. It's a lot quicker to go over the code when you don't need to read as much, similar to how one would write something like x+y=z in math instead of putting descriptive name for each variable in each step or using things "1€/kg" instead of "one euro per kilogram" (or let alone including kilogram's definition from SI standard).
  - lcnPylGDnU4H9OF 3y ago
    Despite this, mathematical proofs still use a bunch of plain words to define things. In a programming context, if one has an “user” from the database, “user” is usually a better name than “u”.
    - IggleSniggle 3y ago
      It really is a context and frequency thing. For example: users.map(u => u.lastName) Nobody is going to have any questions about what 'u' is here. Do that in a codebase that has a 100% consistent and very frequently used User type, and it starts to feel quite reasonable to just use 'u'. It's as familiar as Apple π.
      - lcnPylGDnU4H9OF 3y ago
        This is a perfect example of why I think it’s “usually” better. Still, the word “user” is there in this case. It could be a lot worse: us.map(u => u.lastName); Maybe that’s too close to the word “us”, meaning you and I. We can do better: u_s.map(u => u.lastName); But then is that an abbreviation of something with the initials “U” and “S”? We can do better: users.map(u => u.lastName); Point being that including the word on the line is what provides the clarity. ---- And a bonus option for the truly maniacal: you_guys.map(your => your.lastName);
        syntheweave 3y ago
        I believe a programming language could actually codify rules around how long names are allowed to be based on scope. In a local scope, aliasing to single-letters is OK, and many codebases are littered with i,j,k,b,v,x,y,z arguments and iterators. But putting that into a nested scope has produced many errors for me as I accidentally declare a new n or v and then try to access the outer one. So I'd enforce uniqueness when nesting. After some experience with Pascal, I thought the idea of implicitly reserving "result" as the name for the returned value was a good idea, but also a bit long. Taking a page from TI-BASIC I have settled on "ans" for "answer". Global variables generally have to be a bit longer to not collide. In my own investigations, this happens at four characters: while you can devise some two and three-character abbreviations that work at language level, at four you start getting more complete English words(and English is a bit part of this whole estimation, more densely encoded languages will have their own metrics). Types, functions and classes often cause woes, because they need to be accessed across modules, and modules will use names in linguistically dense ways, but they should not expose that as the default interface in most cases. Here I think the goal should be to have full namespace qualification as the default, and then gradual relaxation near the callsite through explicit aliasing and redeclaration. That last part makes me think that I can go further with what I alias in some languages. Eg. in Haxe, I have the freedom to throw in "typedef Foo = com.corporation.big.fancy.Package.Classname.Method" wherever I like.
- JonChesterfield 3y ago
  Information density matters.
  - deleted 3y ago
    [deleted]
crickey 3y ago
Complains about the craftmans tools rather then the product. IT is rappidly becoming an unusable mess even though every generation of programing languages and frameworks promises revolutions. Honestly i think programing is doomed as man cannot look past the symbolic. Hooked like a lost poet, only complaining about the words used never seing the beauty.
Alupis 3y ago
The issue so very many people fall into is assuming some foreign-looking codebase is awful. What people typically mean to say is something along the lines of "I haven't a clue what this does, haven't enough experience with the language to understand it, or how people use it". Go look at code written in "safe" languages like Rust and tell me you understand what it's doing any better than well written C code. You have to know the language to understand what is sane or not, what the conventions are, what the common issues are, etc. Same with all the Functional stuff out there. To a OO person, it looks bat poo insane. But if you learn FP, it's natural. Some of your complaints are valid from the perspective of someone who doesn't do kernel work. For the people who do work on the kernel, the macros, abbreviations, etc are common place and well understood, and a complete non-issue.
- michaelsalim 3y ago
  I wish people see this more. It's so frustrating to see people pass judgement without even understanding the whole context. Maybe it's due to the lack of documentation sometimes. But many times it's just people not reading them.
- jiggawatts 3y ago
  For reference, I have many years of experience with C, C++, Rust, and even Haskell. That kernel C code still looks awful to me. I've seen beautiful code in every language including C.
- eduction 3y ago
  You’ve circled back to the same point the person you’re replying to was supporting: “ the problem is one of cultural context which is often not shared between generations of coders. “ There is no such thing as absolutely good code. It’s context dependent and context tends to be ephemeral. The Linux kernel is in some ways an exception, but also appreciated by a pretty small group. The rant against the kernel code is meant to be illustrative - if you don’t know the conventions, abbreviations, if you’re not operating in the context of an early 21st century OS, where memory is handled manually, none of this looks particularly great. Code is cultural and cultures are niche and ever changing. Particularly these days. The same technology that enabled the fast rise of, say, Ruby, or Go, or Rust makes your code and my code depreciate quite quickly.
simoncion 3y ago
I bet you a nickel that -if you're a working programmer- most of the functions that you write don't check __every__ possible thing that could go wrong, but -instead- have some assumptions about validations that the caller must run before calling them (e.g. "My callers will hand me well-formed XML"), and impose some constraints on the shape and range of the data that they return to their caller (e.g. "Callers must be prepared to handle a graph that contains cycles."). > ...did I just see a data race... You did see a data race. That race is pretty clearly unavoidable, as it would be triggered by a sysadmin setting the value of a particular sysctl knob "low enough" in between the check mentioned in another function in the chain of function calls that this function is part of being run and the commented code being run. (You can tell that this function part of a chain of function calls because that comment refers to a check in a function that is not called by the function that contains the comment.) > ...okay because it is faster if it’s horrifically unsafe... I mean, here's the call site of the "objectionable" function and the site at which the issue gets detected: <https://github.com/torvalds/linux/blob/master/fs/file.c#L174-L194 https://github.com/torvalds/linux/blob/master/fs/file.c#L174...>. At both ends, there's a comment that says "Hey, it's faster to do this check here, rather than over there.". It's clear that the folks who worked on this code were concerned about the performance, and had discovered that relying on callers to discover if that race had resulted in them getting a smaller allocation than they wanted was needed.
38 3y ago
> It’s written in an unsafe language. it seems you didn't bother to read the entire comment, otherwise you would've seen I linked to TWO different programming languages.
michaelsalim 3y ago
Personally I don't think that's what makes a code "good". All code will get out of date with time. As with everything in this world. Doesn't mean that they are not good. My old gas stove doesn't have the bells and whistles that newer ones does. But it was a really good engineered product that have lasted for decades. It's really well documented so it's easy to do maintenance. It's also well understood by people. Many people will say that an induction stove is better in safety, convenience and many more. But a gas stove still has its own place. Just because the kernel code doesn't use the latest "best practices" doesn't mean it's bad. It could be due to many reasons from performance, compatibility, etc. Software is so easy to update anyway compared to other things that I'd bet there's more "good" software than "good" anything else.
AlotOfReading 3y ago
While I don't think this code is the best, these complaints are like disliking Shakespeare because his writing is archaic. Yeah, no shit. Good writing is relative to the norms and expectations of the audience, which are other kernel developers in this case. It’s written in an unsafe language. So is every other mainstream kernel. Double underscores everywhere, which are the bad alternative to namespaces seen in weak languages like C. It's the designated way to avoid symbol conflicts for system software. Cultural norms. Constants defined with lowercase and not actually marked as const. There aren't any constants in fs/file.c though? Do you mean the sysctls? Those are modifiable at runtime. WTF does “BITBIT_NR(nr)” do!? This is genuinely obscure without background knowledge. fs/file.c maintains a bitmap of bitmaps for performance optimization reasons, hence "bitbit". "nr" has been a standard abbreviation in this part of the kernel for decades. Abbreviations everywhere: ofdr, nfdt, fds, fs, etc… Terse identifiers are just a cultural norm in kernel code. old file descriptor table, new file descriptor table, file descriptors, filesystem, etc... Then there is a long-winded explanation of how they pack bits into an array of longs. Okay, why not make this a reusable module of code? Because C is a weak language, or because the Linux kernel is spaghetti with a dozen implementations of bit maps? Because this code is actually very tricky, performance sensitive, and basically unique in the kernel. The kernel has many things that could be de-duplicated, but I'm confident someone's tried refactoring this and failed for some reason or another, probably performance. Sorry… did I just see a data race just casually commented as “okay because it is faster if it’s horrifically unsafe? You're seeing one of the many design tradeoffs that are made to get good performance in the real world. The VFS code this file is part of is one of the most performance-critical components in the kernel and gets involved with all the other filesystem operations that happen, which on a unix system is essentially everything. The code (and cache footprint) are smaller if the safety burden is pushed off to other people here, which is more important than maintaining an ideal interface. This is some of the most battle-tested code in the world. It's fine if you don't want to modify it, but it's solid code that people have literally bet their lives on given the mildly terrifying use of Linux in safety-critical systems.
- jiggawatts 3y ago
  > disliking Shakespeare because his writing is archaic But... his writing is archaic. Not just quaintly archaic, like a novel from the 1800s, but literally requiring translation archaic. Worse still, the vast, vast majority of people "teaching" Shakespeare pronounce it wrong, which means most of the humour is lost: https://www.youtube.com/watch?v=YiblRSqhL04 https://www.youtube.com/watch?v=YiblRSqhL04 The jokes don't work any more! The rhymes sound wrong! The whole thing is a farce. Theatre. We all pretend it is great English, whe in fact it isn't even English any more. It's taught because our teachers were taught it. Those teachers... and on. It's like the idiots still formatting cloud SSD virtual disks as-if they are physical RAID controllers with spinning rust. It's like the Hungarian notation identifier style people copied from the NT kernel code, even though the NT kernel people realised it was a mistake and moved on. The Linux kernel code would be crazy bad if it wasn't for tens of thousands of people beating on it until it has become merely mediocre. It will never be perfect, it'll never even be "good" code. It has too much inertia, too much history for that to ever happen.
stodor89 3y ago
I've got a different personal perspective: Any code that helps achieve one's ends is good code.

cookiengineer 3y ago

I think I just fell in love with you. Finally a sane person <3 Honestly, I never understood why so many developers abbreviate function names or variables. Is it so much pain to type abc<tab> for auto completion? Why not simply name fd file_descriptor? Short names are a mental barrier and a level of abstraction that can easily be avoided.

sudosysgen 3y ago
In some languages, that leads to terribly long lines that you then have to break down, add indentation, and it just becomes harder to read. There is happy middle.
nemetroid 3y ago
Anyone who has reason to read this file will have "fd" as a word in their vocabulary.
buzer 3y ago
Short names actually help with readability when they are commonly used. It's a lot quicker to go over the code when you don't need to read as much, similar to how one would write something like x+y=z in math instead of putting descriptive name for each variable in each step or using things "1€/kg" instead of "one euro per kilogram" (or let alone including kilogram's definition from SI standard).
- lcnPylGDnU4H9OF 3y ago
  Despite this, mathematical proofs still use a bunch of plain words to define things. In a programming context, if one has an “user” from the database, “user” is usually a better name than “u”.
  - IggleSniggle 3y ago
    It really is a context and frequency thing. For example: users.map(u => u.lastName) Nobody is going to have any questions about what 'u' is here. Do that in a codebase that has a 100% consistent and very frequently used User type, and it starts to feel quite reasonable to just use 'u'. It's as familiar as Apple π.
    - lcnPylGDnU4H9OF 3y ago
      This is a perfect example of why I think it’s “usually” better. Still, the word “user” is there in this case. It could be a lot worse: us.map(u => u.lastName); Maybe that’s too close to the word “us”, meaning you and I. We can do better: u_s.map(u => u.lastName); But then is that an abbreviation of something with the initials “U” and “S”? We can do better: users.map(u => u.lastName); Point being that including the word on the line is what provides the clarity. ---- And a bonus option for the truly maniacal: you_guys.map(your => your.lastName);
      - syntheweave 3y ago
        I believe a programming language could actually codify rules around how long names are allowed to be based on scope. In a local scope, aliasing to single-letters is OK, and many codebases are littered with i,j,k,b,v,x,y,z arguments and iterators. But putting that into a nested scope has produced many errors for me as I accidentally declare a new n or v and then try to access the outer one. So I'd enforce uniqueness when nesting. After some experience with Pascal, I thought the idea of implicitly reserving "result" as the name for the returned value was a good idea, but also a bit long. Taking a page from TI-BASIC I have settled on "ans" for "answer". Global variables generally have to be a bit longer to not collide. In my own investigations, this happens at four characters: while you can devise some two and three-character abbreviations that work at language level, at four you start getting more complete English words(and English is a bit part of this whole estimation, more densely encoded languages will have their own metrics). Types, functions and classes often cause woes, because they need to be accessed across modules, and modules will use names in linguistically dense ways, but they should not expose that as the default interface in most cases. Here I think the goal should be to have full namespace qualification as the default, and then gradual relaxation near the callsite through explicit aliasing and redeclaration. That last part makes me think that I can go further with what I alias in some languages. Eg. in Haxe, I have the freedom to throw in "typedef Foo = com.corporation.big.fancy.Package.Classname.Method" wherever I like.
JonChesterfield 3y ago
Information density matters.
- deleted 3y ago
  [deleted]

crickey 3y ago

Complains about the craftmans tools rather then the product. IT is rappidly becoming an unusable mess even though every generation of programing languages and frameworks promises revolutions. Honestly i think programing is doomed as man cannot look past the symbolic. Hooked like a lost poet, only complaining about the words used never seing the beauty.

Alupis 3y ago

The issue so very many people fall into is assuming some foreign-looking codebase is awful. What people typically mean to say is something along the lines of "I haven't a clue what this does, haven't enough experience with the language to understand it, or how people use it". Go look at code written in "safe" languages like Rust and tell me you understand what it's doing any better than well written C code. You have to know the language to understand what is sane or not, what the conventions are, what the common issues are, etc. Same with all the Functional stuff out there. To a OO person, it looks bat poo insane. But if you learn FP, it's natural. Some of your complaints are valid from the perspective of someone who doesn't do kernel work. For the people who do work on the kernel, the macros, abbreviations, etc are common place and well understood, and a complete non-issue.

michaelsalim 3y ago
I wish people see this more. It's so frustrating to see people pass judgement without even understanding the whole context. Maybe it's due to the lack of documentation sometimes. But many times it's just people not reading them.
jiggawatts 3y ago
For reference, I have many years of experience with C, C++, Rust, and even Haskell. That kernel C code still looks awful to me. I've seen beautiful code in every language including C.
eduction 3y ago
You’ve circled back to the same point the person you’re replying to was supporting: “ the problem is one of cultural context which is often not shared between generations of coders. “ There is no such thing as absolutely good code. It’s context dependent and context tends to be ephemeral. The Linux kernel is in some ways an exception, but also appreciated by a pretty small group. The rant against the kernel code is meant to be illustrative - if you don’t know the conventions, abbreviations, if you’re not operating in the context of an early 21st century OS, where memory is handled manually, none of this looks particularly great. Code is cultural and cultures are niche and ever changing. Particularly these days. The same technology that enabled the fast rise of, say, Ruby, or Go, or Rust makes your code and my code depreciate quite quickly.

simoncion 3y ago

I bet you a nickel that -if you're a working programmer- most of the functions that you write don't check __every__ possible thing that could go wrong, but -instead- have some assumptions about validations that the caller must run before calling them (e.g. "My callers will hand me well-formed XML"), and impose some constraints on the shape and range of the data that they return to their caller (e.g. "Callers must be prepared to handle a graph that contains cycles."). > ...did I just see a data race... You did see a data race. That race is pretty clearly unavoidable, as it would be triggered by a sysadmin setting the value of a particular sysctl knob "low enough" in between the check mentioned in another function in the chain of function calls that this function is part of being run and the commented code being run. (You can tell that this function part of a chain of function calls because that comment refers to a check in a function that is not called by the function that contains the comment.) > ...okay because it is faster if it’s horrifically unsafe... I mean, here's the call site of the "objectionable" function and the site at which the issue gets detected: <https://github.com/torvalds/linux/blob/master/fs/file.c#L174-L194 https://github.com/torvalds/linux/blob/master/fs/file.c#L174...>. At both ends, there's a comment that says "Hey, it's faster to do this check here, rather than over there.". It's clear that the folks who worked on this code were concerned about the performance, and had discovered that relying on callers to discover if that race had resulted in them getting a smaller allocation than they wanted was needed.

38 3y ago

> It’s written in an unsafe language. it seems you didn't bother to read the entire comment, otherwise you would've seen I linked to TWO different programming languages.

michaelsalim 3y ago

Personally I don't think that's what makes a code "good". All code will get out of date with time. As with everything in this world. Doesn't mean that they are not good. My old gas stove doesn't have the bells and whistles that newer ones does. But it was a really good engineered product that have lasted for decades. It's really well documented so it's easy to do maintenance. It's also well understood by people. Many people will say that an induction stove is better in safety, convenience and many more. But a gas stove still has its own place. Just because the kernel code doesn't use the latest "best practices" doesn't mean it's bad. It could be due to many reasons from performance, compatibility, etc. Software is so easy to update anyway compared to other things that I'd bet there's more "good" software than "good" anything else.

AlotOfReading 3y ago

While I don't think this code is the best, these complaints are like disliking Shakespeare because his writing is archaic. Yeah, no shit. Good writing is relative to the norms and expectations of the audience, which are other kernel developers in this case. It’s written in an unsafe language. So is every other mainstream kernel. Double underscores everywhere, which are the bad alternative to namespaces seen in weak languages like C. It's the designated way to avoid symbol conflicts for system software. Cultural norms. Constants defined with lowercase and not actually marked as const. There aren't any constants in fs/file.c though? Do you mean the sysctls? Those are modifiable at runtime. WTF does “BITBIT_NR(nr)” do!? This is genuinely obscure without background knowledge. fs/file.c maintains a bitmap of bitmaps for performance optimization reasons, hence "bitbit". "nr" has been a standard abbreviation in this part of the kernel for decades. Abbreviations everywhere: ofdr, nfdt, fds, fs, etc… Terse identifiers are just a cultural norm in kernel code. old file descriptor table, new file descriptor table, file descriptors, filesystem, etc... Then there is a long-winded explanation of how they pack bits into an array of longs. Okay, why not make this a reusable module of code? Because C is a weak language, or because the Linux kernel is spaghetti with a dozen implementations of bit maps? Because this code is actually very tricky, performance sensitive, and basically unique in the kernel. The kernel has many things that could be de-duplicated, but I'm confident someone's tried refactoring this and failed for some reason or another, probably performance. Sorry… did I just see a data race just casually commented as “okay because it is faster if it’s horrifically unsafe? You're seeing one of the many design tradeoffs that are made to get good performance in the real world. The VFS code this file is part of is one of the most performance-critical components in the kernel and gets involved with all the other filesystem operations that happen, which on a unix system is essentially everything. The code (and cache footprint) are smaller if the safety burden is pushed off to other people here, which is more important than maintaining an ideal interface. This is some of the most battle-tested code in the world. It's fine if you don't want to modify it, but it's solid code that people have literally bet their lives on given the mildly terrifying use of Linux in safety-critical systems.

jiggawatts 3y ago
> disliking Shakespeare because his writing is archaic But... his writing is archaic. Not just quaintly archaic, like a novel from the 1800s, but literally requiring translation archaic. Worse still, the vast, vast majority of people "teaching" Shakespeare pronounce it wrong, which means most of the humour is lost: https://www.youtube.com/watch?v=YiblRSqhL04 https://www.youtube.com/watch?v=YiblRSqhL04 The jokes don't work any more! The rhymes sound wrong! The whole thing is a farce. Theatre. We all pretend it is great English, whe in fact it isn't even English any more. It's taught because our teachers were taught it. Those teachers... and on. It's like the idiots still formatting cloud SSD virtual disks as-if they are physical RAID controllers with spinning rust. It's like the Hungarian notation identifier style people copied from the NT kernel code, even though the NT kernel people realised it was a mistake and moved on. The Linux kernel code would be crazy bad if it wasn't for tens of thousands of people beating on it until it has become merely mediocre. It will never be perfect, it'll never even be "good" code. It has too much inertia, too much history for that to ever happen.

stodor89 3y ago

I've got a different personal perspective: Any code that helps achieve one's ends is good code.