this one made me think on improvement refactorings that always break something, even though the old code needed improvement. Specially just before releases.
Agreed! Do you wait until the Friday afternoon right before your DevOps lead is scheduled to leave for a well-deserved vacation, too? In my experience it really takes things to a whole new level of awesome!
If one's CI does not have a pre-production stage with integration tests, automatic blockers, and automatic rollback, one should rethink their CI design.
Yes, but the point is that sort of change requires _REALLY_ good unit tests.
Do you have a check that every place where an integer is parsed from a string will correctly handle the error of the string not being an integer within the required range?
It seems like a good fuzzer would catch this - it's pretty common for fuzzers to stick strangely formatted (negative, out-of-range, octal, etc.) numbers in input text.
if (isdigit(*str)) {
- io_tlb_nslabs = simple_strtoul(str, &str, 0);
+ str += parse_integer(str, 0, &io_tlb_nslabs);
and obviously nobody spent even a *second* asking themselves
"what if parse_integer returns an error".
Can anybody explain why parse_integer would return an error, when isdigit(*str)?
Also, why is the result (presumably integer) added back to the string?
EDIT: Regarding to "adding to the string" I realize that's just C pointer arithmetic.
Not sure because I didn't read the parse_integer patches. But the converse is a problem. isdigit("-2") returns false but the value will parse correctly.
An overflow may happen if there is an overflow for the type (e.g. reading '300' into an unsigned char).
The function returns either a positive number indicating the number of characters successfully read from the string (if it encountered a non-digit character) or a negative number indicating an error code.
Haven't read any of the relevant code, but if the new code returns a negative number representing an error, it will decrement the `str` pointer, whereas I'm guessing `simple_strtoul` leaves its `str` pointer alone on error.
I don't know about Linux's simple_strtoul(), but the C library's strtoul(3) will return ULONG_MAX and set the string pointer to a valid location. So the old code COULD cause an issue if "io_tlb_nslabs" being equal to ULONG_MAX isn't accounted for later on, but adding -1 (0xffffffffffffffff) to a pointer (as the new code does) is far worse.
I haven't looked at the patch, but the error is probably overflow: too many digits to fit in the integer variable.
parse_integer() presumably returns the number of bytes consumed, so the str pointer is advanced by that many bytes to point to whatever is after the integer. But on error, it returns a negative number, so str is moved backwards into some nonsensical position.
> And yes, parse_integer() really can return an error, even despite that "isdigit(*str)" check. Think about it. Or just read the source code.
Actually taking the advice of the article and looking at the source code makes it clear that parse_integer is doing what we would hope such an API would: it guarantees that the resulting number fits into the provided integer.
Returning an error is fine, but the code that calls the function doesn't check for errors, and just blindly applies the return value as an offset to a pointer. When the function returns -ERANGE to indicate that the value was too large for the requested data type, entertaining things will occur.
IMHO, "leading zero indicates octal" is as old as it is wrong. There are a handful of cases where this is helpful, and an ocean of cases where it breaks things unexpectedly because someone tried inputting prettyprinted data.
At least with Hex you can't mistake the 0x prefix for anything else.
One of the best features of inet_pton over inet_addr is that the former doesn't do automatic conversions and instead assumes every field is a decimal number. The latter has a hidden trap where 192.168.0.11 is different than 192.168.000.011.
It is the specified behavior of the discussed code when not passing in a specified base to parse_integer. Now if that is a needed feature in the example context (or a fixed base of 10 would work as well) I don't know. It is an odd convention though, I agree. Hex I somewhat get, but octal?
Yeah, so it's a wrong thing in 2015 where the only significant use of octal is in unix permissions which themselves are only octal because of PDP-7/8.
The 9/18/36-bits architectures have lost but octal conventions of C was made on the PDP-7 (18 bits) and until the early 1980s, 36-bit machines were commercially available (PDP-10.)
If isdigit() is the standard library function, that takes/checks a single character. However, parse_integer() takes a full string and parses until it reaches a null or newline. The first character could definitely be a valid digit but the rest of the string could cause a problem. However, I am not a C programmer, so my internet search-guided intuition may be wrong.
Much as I like git, its inconsistencies wind me up sometimes.
Anything that's not very commonly used or immediately obvious tends to be buried in some arcane option, and not always an option to a command you'd expect.
This is not a non-sequitur: Linus might have thought good things while first implementing git, but it's one example of a tool that I use on the command line but has many, many alternative UIs that all claim to be better (not just graphical, even CLI) interfaces.
Seems to me that it became popular because it was better than other popular options at the time like CVS and SVN. I use it today because of a choice I made in the past, and I can use it more effectively today than hg.
Git is popular because when GitHub was in full startup mode (and not the current $350 million in cash, sit-on-their-hands mode), they ran bar meetups and gave every ruby novice in the world free alcohol to join their platform.
Turns out free booze times tens of thousands of initial users beats the better interface of hg being advertised on impersonal mailing lists.
At this point, (experienced) people have just gotten used to all the weird edge cases and overly complex ways of doing things with git, so changing back to a simpler way is now more complicated.
The linux project was using git before github even existed, and would probably still use it even if the rest of the world had chosen, say, bazaar instead.
Yes, but that's not what made git popular. What revision control did Linux use before git? It sure wasn't popular with the public even though Linux used it.
What made git popular was thousands of slightly clueless novice Ruby users forced on to git by GitHub. They then began writing thousands of blog posts about how git is confusing, but—they, in their infinite cleverness—discovered the secrets and will now share them with you.
Yep, it was Github rather than git. Rails being hosted on Github early on was a big factor too. The rest of the Ruby community followed. Then came about 3/4s of the Python community (some big holdouts still on hg though), then the fast growing Javascript community etc. The network effect of Github collaboration snowballed it in a way a standalone source control tool couldn't.
In those early Github days the Rails community had the most effective evangelism and hipster clout. The "modern git" vs "crufty svn" narrative back then closely paralleled the "modern Ruby/Rails" vs "crufty Java" narrative too.
The Linux kernel community was never going to drive widespread git adoption by itself - the average coder would never do any kernel coding. At the time, the core Linux devs only cared about making a tool for themselves, and they still don't like the way Github works.
It really bugs me that "refactoring" has been taken to mean any code change (hopefully) without end-user visible effect. We didn't need a new word for that. Refactoring is about doing that in a rigously reliable way.
My rule is, if you don't have good tests, you aren't "refactoring" you're "moving shit around and praying you don't break something".
My rule is: tests only check the presence of bugs, not their absence. And is usually little more than wishful thinking to delude people into believing their code to be bug-free.
Understanding the code is worth WAY more than having good tests. Most of the great programmers who inspire me don't write tests but have the deepest understanding of whats going on.
I've seen far more problems from people who think they understand the code so they don't need tests than from people who write tests to check their code. The best of the best might be able to get by without tests, but us common-folk stand to benefit a lot from writing tests and listening to the design feedback they provide.
While I agree with the general idea in theory, I haven't seen it happen that often in practice.
More often than not tests will constrain the architecture of the actual system and result in code that is much more complex than what is required to solve the problem at hand. People then start having blind faith in their tests while being completely oblivious to what they don't test. At that point the codebase is such a mess that you do need tests to maintain it.
People who focus on understanding their code work very, very hard to keep their codebase simple enough to quickly wrap their heads around it. I know I modify my codebase way too much for it to have tests - I'd spend more time refactoring the tests than progressing and I wouldn't be able to simplify the architecture as much because my attention is now on how testable the code is rather than how simple it is.
I see projects shipped with unittests that still required maintenance and debugging 6 months down the road, and others without tests that worked flawlessly out the door.
Of course, there's a difference between understanding and thinking you understand, but that's another matter entirely as we're walking into Dunning-Kruger territory!
I'm not saying tests are bad! Far from it, I do write tests for functions where the logic isn't obvious, but my point is that there's too much focus on testing and not enough on actual quality and simplicity.
Sounds like a lot of projects you worked on had tests at the wrong granularity.
Fine grained unit tests are good for helping you find out where a bug is in the code, at the expense of calcifying your architecture. "Functional", "integration", "end-to-end" or other higher level tests don't pinpoint a bug as easily but in return they give you greater freedom to reorganize your implementation.
It has a metric ton of tests, which is absolutely critical for ensuring a change to one corner of the formatting doesn't regress other things. Formatting is solving a bunch of global constraints, so almost every change interacts with other things.
I've rewritten almost the whole thing twice at this point, and the tests never slowed me down. That's because they're functional tests: they just run the formatter and validate the expected output.
It does mean I spent a hell of a lot more time figuring out why some test was failing, but it made refactoring a joy.
Everyone thinks they understand what they are implementing but then inevitably encounter some unexpected "feature". Tests help, but yeah... halting problem.
As for those great programmers how many sloc can they write without any compilation error and have the application executing as intended? If they make mistakes then bugs can also happen.
> Most of the great programmers who inspire me don't write tests but have the deepest understanding of whats going on.
Understanding the code and having tests is not mutually exclusive.
And if you have ever inherit a code base from somebody who "didn't need to write tests", because he or she understood the code so well, you will come to appreciate tests in a whole new way. ;-)
Refactoring is abused as a handy word to use to describe random shit that would confuse management if you went into detail. If you're genuinely refactoring then, depending on the scope of the refactoring and the nature of the code being refactored, I would agree. Making that judgment is part of being good at your job.
But "refactoring" often means "cleaning up some really shitty code we wrote in haste last month now that we aren't facing a horrible deadline", and that's worth doing with or without good tests. And using a nice opaque term like refactoring stops your having to explain "good, fast, cheap" to some manager like he/she is stupid (which he/she is not) for the umpteenth time.
I'm curious what Linux has against the strl* functions. He says that they can be implemented wrong too, but it seems like they are harder to screw up than the strn* versions that required you to manually terminate the string when it hits maximum length.
>the strn* versions that required you to manually terminate the string when it hits maximum length
This is from the description of the not-easy-to-use strncat():
>A terminating null character is always appended to the result.
So your statement is only true for strncpy(). Unlike many other C functions it doesn't deal with C strings (null-terminated character arrays), but rather with fixed length and possibly null padded character arrays. People are just misusing it for some false sense of security. It may seem strange but snprintf() is the only secure string copy function defined by ISO C99.
That doesn't explain anything. The manpage explains the problem though:
RETURN VALUES
The strlcpy() and strlcat() functions return the total length of the
string they tried to create. For strlcpy() that means the length of src.
For strlcat() that means the initial length of dst plus the length of
src. While this may seem somewhat confusing, it was done to make trunca-
tion detection simple.
So strlcpy/strlcat parse all of src regardless of what you pass on that third parameter. And they'll happily run off the end of the buffer if your string isn't null terminated. I see what Linus was so mad about now. All because they want to return the length of the original string instead of the length of the copied string.
The underlying problem is that C is too limited in return types from functions. There should really be a fourth (optional) flag to strncpy that is set if the string was truncated to fit in the new buffer, and the function would return the number of characters copied, not the number of characters in the source string.
“People who implement new and improved interfaces always seem to get that wrong.”
Linus is completely right on this. I have seen this over and over again where developers, creative and problem solving as they are, attack things with their limited experience and knowledge and fail to understand things fully before heading in like a bull.
There is a time and place for that sort of behavior, though. Linus wrote Linux with that same mentality. Sure, he was exponentially smarter and more able. :) But, that was the time for starting fresh with a cavalier attitude. However, if you try to contribute something like this to Linux today, as mature and stable as it is, and majorly screw it up, yes, you are wrong and expect your ass to get chewed.
I just worry for Linux's sake who, if anyone, will be able to replace Linus if he were to choose to retire or were whisked away by aliens (I won't say die). Some related discussions about that:
Also, understanding which problems are worth solving is hard as well. That's where the "charge in like a bull" approach can be handy: a number of problems are worth solving that people shy away from, either because they're viewed as too inconsequential or because they're viewed as too difficult, and they don't get resources devoted to them until somebody has paved the way with a quick & dirty solution.
I found a quote recently by G.K. Chesterton about this that I really like.
"In the matter of reforming things, as distinct from deforming them, there is one plain and simple principle; a principle which will probably be called a paradox. There exists in such a case a certain institution or law; let us say, for the sake of simplicity, a fence or gate erected across a road. The more modern type of reformer goes gaily up to it and says, “I don’t see the use of this; let us clear it away.” To which the more intelligent type of reformer will do well to answer: “If you don’t see the use of it, I certainly won’t let you clear it away. Go away and think. Then, when you can come back and tell me that you do see the use of it, I may allow you to destroy it."[1]
Nobody builds a fence for no reason. Keep in mind that he isn't implying that it was a good reason, just that there's a reason why it's there and you should understand that reason before taking it down.
I can't get to completely agree with it, but the completely opposite idea that you should just not care is stupid.
It appears to be less true now than in days past, but humans are -first and foremost- storytellers. we like to tell stories, we like to listen to stories, and we -often- better remember lessons imparted in the form of a story.
Moreover, if you're trying to alter someone's opinion about the truth of a matter, you almost always need bolster your core argument with supplemental material. You often don't know much about your listener's background, you probably don't know what motivates him, and he may very well be opposed to your core argument for entirely unreasonable reasons. It's often best to surround your core argument with a large, somewhat varied structure, so that many folks will find some way to latch on to it.
> solving problems is easy, it's understanding what the problem is that's hard.
I completely agree. These days, I work as a sysadmin/helpdesk monkey, and I am having this experience pretty much all the time - once I understand what is wrong, the solution becomes pretty obvious. Finding out what is wrong, though, can be really tricky. (More so on Windows - troubleshooting on Windows often enough ends in "Let's just reboot and hope it works next time")
> solving problems is easy, it's understanding what the problem is that's hard
Great way to lay it out. How many hours have I spent debugging just to come to the conclusion that the code does everything according to the requirements — it's the requirements which are buggy.
I agree with the basic and maybe its specially true for a project like linux kernel but there is another side to it too.
I usually refactor after understanding the code specially oddball sections which usually means it's handling some non trivial corner case. Now there has been a fair amount of cases where a rotten code has survived and caused other bits of rotten codes to be added because everyone keeps thinking it must have a purpose when it really doesn't. It is a hard problem but sometimes you have to take a bit of risk specially if the current code/interface keep decreasing the quality of all the new codes surrounding it.
And it's one of the reason when reviewing code I am extra hard on code that solves problem in a complicated way or do extra pointless work since those can cause these type of confusion in future.
I actually did a fair amount of this at work in the latest release. It was a combination of business rule change, data structure change and since it was the core security model we also did major interface change (the one linus is mentioning) so that the rest of the app/developer can work on a simpler/easier model. I spend more days discussing and planning the change then the actual coding for once and despite the few bugs at QA it had been a success. It had been one of the biggest change in my 10 years of work and one of the most stable. My takeaway from that is to iterate over the design a lot then iterate over the code a lot before really pushing it everywhere and probably have some break in between (if it's important enough) to have multiple go at it with fresh perspective.
This reminds of a piece of code I saw during my training, a web application written in Perl, by a Java guy, that I had to knead into shape.
When I was almost finished with it, the one ugly element that remained was a C-style for loop with a head that filled one and a half lines (about 150-160 characters) of seriously cryptic code, and its body was not much better.
I had been explicitly ignoring this loop while I was working on the application, because I had no clue what it did, and the app was working more or less they way it should.
But when I was finished slightly ahead of schedule, I could not help but use the free time I had left trying to figure out what the for-loop did. Otherwise, the code I had been massaging for a couple of months now was pretty pretty, but the prettier it had become, the more this ugly for-loop stuck out.
I started by adding a simple logging statement to its body that simply announced the fact that the loop was executing an iteration. Then I played around with the application for a while. Eventually, I took a look at the log file to see how often the loop had executed, and was rather baffled - the loop had executed a total of zero times.
I played around with the app some more, trying my worst to make that loop run at least once (which was, admittedly, hard, because I had no clue what it was for). Two hours later, I gave up and removed the loop.
I finished my training about eight months later, and at least during that time, I did not hear of it again. (I really would have liked to know where that loop came from, though.)
"I played around with the app some more, trying my worst to make that loop run at least once (which was, admittedly, hard, because I had no clue what it was for). Two hours later, I gave up and removed the loop."
That seems very risky, and I'd never have allowed that change to pass a code review. Just because you couldn't figure out how to make the loop execute doesn't mean that there aren't any paths through the code that would cause it to execute. The only safe way to remove it would be to prove that it can't ever execute -- e.g., the loop only executes if the variable foo is true (the condition is "foo && ..."), but foo is initialized to false and isn't modified anywhere else in the code.
>The more modern type of reformer goes gaily up to it and says, “I don’t see the use of this; let us clear it away.” To which the more intelligent type of reformer will do well to answer: “If you don’t see the use of it, I certainly won’t let you clear it away. Go away and think. Then, when you can come back and tell me that you do see the use of it, I may allow you to destroy it.
In general true but sometimes it's almost impossible to prove that it is actually unwanted or plain wrong and you need to take some risk for the bigger benefit. It's not something to do lightly but it is actually needed sometimes. Our application has bits that hasn't been touched in years and no devel exists with experience in the said (problematic code) and if new development is needed and if that bit puts the new work at risk sometimes you have to make some judgement call and take the calculated risk.
First of all, I was a trainee. I do not know if that ever even entered production use. Secondly, the problem the web application was intended to solve was really trivial: Service managers used it to register customer machines that were intended to send email; they would enter the IP address of the server, the DNS domain of the sender and the account number of the customer to bill. The web application validated the data (fairly crudely - the spec I was given defined an account number as "consisting of decimal digits", and an email address as "containing an @-sign"), entered it into a database and sent an email to the firewall team so they would adjust their rule sets. Me and my supervisor looked at that loop intensely, and there was no explanation we could come up with why it would even be there in the first place.
(The whole thing was less than a thousand lines of not-very-dense Perl code, plus a few hundred lines of HTML templates. It's not like this was some highly complex piece of code.)
This got me thinking how much test coverage exist for the kernel. It sounds like most of those issues could've been caught by tests (and I'm not referring to API compatibility, but purely to the behavior of the implementation)
> We had similar issues with the completely moronic and misdesigned crap called "strlcpy()", which was introduced for similar reasons, and also caused nasty bugs where the old code was actually correct, and the "converted to better and safer interfaces" code was actually buggy.
His issue was converting a whole lot of code over to strlcpy() while introducing bugs that didn't previously exist.
For another example of this kind of irrational thinking, look at NeoVim's massive churning changes to the Vim codebase for the purposes of "modernization".
I didn't track the style of Neovim changes, but I appreciate them actually making a moving project. vim has bugs and feature requests which aren't fixed for years.
I'm using Neovim currently and really enjoy that it supports stuff like 24 bit color schemes in the terminal. I finally can use color schemes which don't hurt my eyes.
strlcpy() always returns the length of the src str, even if it does not fit and gets truncated with a NULL. My hunch is strncpy() and strlcpy() calls were replaced with strlcpy() calls and whatever code where was before to potentially handle the not fitting and not adding a NULL was removed. Then no code was added after to verify it fit. Imagine it just incremented dst by the return value and then called strlcpy() with the next src str to glue together...
EDIT: Or there is no NULL in src and it runs off into an unmapped page calculating the len :D Wow it's just a pretty bad idea for a lot of cases the more I think about it.
Don't know why you are voted down, string processing is indeed unpleasantly difficult in C. Partly because a built-in string type would conflict with C's simplistic compile/link schema.
Have there been any serious attempts at an alternate string library interface for C, perhaps even an total redo of the standard C library with decades of hindsight?
Custom string libraries for C are plenty. Pretty much all large C projects accumulate extensive libraries of utility functions, and string processing is a common one to abstract.
There have been /many/ attempts. Here's a link to a comment on the sds from redis post (yet another alternative string library) that links to a nice overview of a number of alternatives:
Thanks mzs for that great reference to an old HN comment. Appreciated!
The volume of string libraries only suggests how crucial core library design, often an afterthought, is to programming language design. The language itself encourages idioms, and it's for the core library to harness these idioms into a robust, cohesive, comprehensive toolbox for common tasks.
I just recently discovered that modern C actually has syntax for forcing sized arrays as function arguments. You can do:
foo(size_t n, char arr[restrict static n])
The static qualifier here means "no really, I actually want the compiler to complain if the passed array isn't the size indicated". (because C doesn't have enough different meanings for the word "static" :( ). I haven't seen this in the wild yet or checked if gcc and clang actually implement it usefully, but it seems like it could be quite useful.
can unit tests be written for every use case that's known to work with the old method, and then used on every new replacement? People wouldn't even get to submit design changes like this unless it passes all known use cases.
129 comments
[ 3.5 ms ] story [ 200 ms ] threadCredit where due: I stole that from a co-worker named Kalen Braley. (He doesn't actually do that, as far as I know.)
how is it going to be pushed to production if it fails tests ? :P
Do you have a check that every place where an integer is parsed from a string will correctly handle the error of the string not being an integer within the required range?
Also, why is the result (presumably integer) added back to the string?
EDIT: Regarding to "adding to the string" I realize that's just C pointer arithmetic.
The function returns either a positive number indicating the number of characters successfully read from the string (if it encountered a non-digit character) or a negative number indicating an error code.
https://lwn.net/Articles/643159/
parse_integer() presumably returns the number of bytes consumed, so the str pointer is advanced by that many bytes to point to whatever is after the integer. But on error, it returns a negative number, so str is moved backwards into some nonsensical position.
Actually taking the advice of the article and looking at the source code makes it clear that parse_integer is doing what we would hope such an API would: it guarantees that the resulting number fits into the provided integer.
https://lwn.net/Articles/643159/
But that is parsed as "0" and the 9 is left in the string.
At least with Hex you can't mistake the 0x prefix for anything else.
One of the best features of inet_pton over inet_addr is that the former doesn't do automatic conversions and instead assumes every field is a decimal number. The latter has a hidden trap where 192.168.0.11 is different than 192.168.000.011.
The 9/18/36-bits architectures have lost but octal conventions of C was made on the PDP-7 (18 bits) and until the early 1980s, 36-bit machines were commercially available (PDP-10.)
Anything that's not very commonly used or immediately obvious tends to be buried in some arcane option, and not always an option to a command you'd expect.
This is not a non-sequitur: Linus might have thought good things while first implementing git, but it's one example of a tool that I use on the command line but has many, many alternative UIs that all claim to be better (not just graphical, even CLI) interfaces.
http://git-scms.com/
Turns out free booze times tens of thousands of initial users beats the better interface of hg being advertised on impersonal mailing lists.
At this point, (experienced) people have just gotten used to all the weird edge cases and overly complex ways of doing things with git, so changing back to a simpler way is now more complicated.
What made git popular was thousands of slightly clueless novice Ruby users forced on to git by GitHub. They then began writing thousands of blog posts about how git is confusing, but—they, in their infinite cleverness—discovered the secrets and will now share them with you.
In those early Github days the Rails community had the most effective evangelism and hipster clout. The "modern git" vs "crufty svn" narrative back then closely paralleled the "modern Ruby/Rails" vs "crufty Java" narrative too.
The Linux kernel community was never going to drive widespread git adoption by itself - the average coder would never do any kernel coding. At the time, the core Linux devs only cared about making a tool for themselves, and they still don't like the way Github works.
How many lessons we seem to have to learn the hard way in our careers that others before us have learned: https://en.wikipedia.org/wiki/The_Mythical_Man-Month#The_sec...
http://www.joelonsoftware.com/articles/fog0000000069.html
My rule is, if you don't have good tests, you aren't "refactoring" you're "moving shit around and praying you don't break something".
Understanding the code is worth WAY more than having good tests. Most of the great programmers who inspire me don't write tests but have the deepest understanding of whats going on.
More often than not tests will constrain the architecture of the actual system and result in code that is much more complex than what is required to solve the problem at hand. People then start having blind faith in their tests while being completely oblivious to what they don't test. At that point the codebase is such a mess that you do need tests to maintain it.
People who focus on understanding their code work very, very hard to keep their codebase simple enough to quickly wrap their heads around it. I know I modify my codebase way too much for it to have tests - I'd spend more time refactoring the tests than progressing and I wouldn't be able to simplify the architecture as much because my attention is now on how testable the code is rather than how simple it is.
I see projects shipped with unittests that still required maintenance and debugging 6 months down the road, and others without tests that worked flawlessly out the door.
Of course, there's a difference between understanding and thinking you understand, but that's another matter entirely as we're walking into Dunning-Kruger territory!
I'm not saying tests are bad! Far from it, I do write tests for functions where the logic isn't obvious, but my point is that there's too much focus on testing and not enough on actual quality and simplicity.
Fine grained unit tests are good for helping you find out where a bug is in the code, at the expense of calcifying your architecture. "Functional", "integration", "end-to-end" or other higher level tests don't pinpoint a bug as easily but in return they give you greater freedom to reorganize your implementation.
For example, I work on a code formatter:
https://github.com/dart-lang/dart_style
It has a metric ton of tests, which is absolutely critical for ensuring a change to one corner of the formatting doesn't regress other things. Formatting is solving a bunch of global constraints, so almost every change interacts with other things.
I've rewritten almost the whole thing twice at this point, and the tests never slowed me down. That's because they're functional tests: they just run the formatter and validate the expected output.
It does mean I spent a hell of a lot more time figuring out why some test was failing, but it made refactoring a joy.
As for those great programmers how many sloc can they write without any compilation error and have the application executing as intended? If they make mistakes then bugs can also happen.
It's only a matter of code complexity.
Understanding the code and having tests is not mutually exclusive.
And if you have ever inherit a code base from somebody who "didn't need to write tests", because he or she understood the code so well, you will come to appreciate tests in a whole new way. ;-)
The programmers who inspire me deeply understand their code and their own fallibility so they also write tests.
But "refactoring" often means "cleaning up some really shitty code we wrote in haste last month now that we aren't facing a horrible deadline", and that's worth doing with or without good tests. And using a nice opaque term like refactoring stops your having to explain "good, fast, cheap" to some manager like he/she is stupid (which he/she is not) for the umpteenth time.
This is from the description of the not-easy-to-use strncat():
>A terminating null character is always appended to the result.
So your statement is only true for strncpy(). Unlike many other C functions it doesn't deal with C strings (null-terminated character arrays), but rather with fixed length and possibly null padded character arrays. People are just misusing it for some false sense of security. It may seem strange but snprintf() is the only secure string copy function defined by ISO C99.
RETURN VALUES The strlcpy() and strlcat() functions return the total length of the string they tried to create. For strlcpy() that means the length of src. For strlcat() that means the initial length of dst plus the length of src. While this may seem somewhat confusing, it was done to make trunca- tion detection simple.
So strlcpy/strlcat parse all of src regardless of what you pass on that third parameter. And they'll happily run off the end of the buffer if your string isn't null terminated. I see what Linus was so mad about now. All because they want to return the length of the original string instead of the length of the copied string.
The underlying problem is that C is too limited in return types from functions. There should really be a fourth (optional) flag to strncpy that is set if the string was truncated to fit in the new buffer, and the function would return the number of characters copied, not the number of characters in the source string.
The different return is more like what MS does with the _s routines, though it uses a special length return and flag option.
Linus is completely right on this. I have seen this over and over again where developers, creative and problem solving as they are, attack things with their limited experience and knowledge and fail to understand things fully before heading in like a bull.
There is a time and place for that sort of behavior, though. Linus wrote Linux with that same mentality. Sure, he was exponentially smarter and more able. :) But, that was the time for starting fresh with a cavalier attitude. However, if you try to contribute something like this to Linux today, as mature and stable as it is, and majorly screw it up, yes, you are wrong and expect your ass to get chewed.
I just worry for Linux's sake who, if anyone, will be able to replace Linus if he were to choose to retire or were whisked away by aliens (I won't say die). Some related discussions about that:
http://ubuntuforums.org/archive/index.php/t-424721.html
https://www.reddit.com/r/linux/comments/2fnnva/what_would_ha...
http://www.jwz.org/doc/cadt.html
Over the years I gradually came to this core truth of programming: solving problems is easy, it's understanding what the problem is that's hard.
Kinda sorta similar to "it's not about coding, it's about modelling": http://www.chris-granger.com/2015/01/26/coding-is-not-the-ne...
"In the matter of reforming things, as distinct from deforming them, there is one plain and simple principle; a principle which will probably be called a paradox. There exists in such a case a certain institution or law; let us say, for the sake of simplicity, a fence or gate erected across a road. The more modern type of reformer goes gaily up to it and says, “I don’t see the use of this; let us clear it away.” To which the more intelligent type of reformer will do well to answer: “If you don’t see the use of it, I certainly won’t let you clear it away. Go away and think. Then, when you can come back and tell me that you do see the use of it, I may allow you to destroy it."[1]
[1] http://www.chesterton.org/taking-a-fence-down/
I can't get to completely agree with it, but the completely opposite idea that you should just not care is stupid.
Moreover, if you're trying to alter someone's opinion about the truth of a matter, you almost always need bolster your core argument with supplemental material. You often don't know much about your listener's background, you probably don't know what motivates him, and he may very well be opposed to your core argument for entirely unreasonable reasons. It's often best to surround your core argument with a large, somewhat varied structure, so that many folks will find some way to latch on to it.
I completely agree. These days, I work as a sysadmin/helpdesk monkey, and I am having this experience pretty much all the time - once I understand what is wrong, the solution becomes pretty obvious. Finding out what is wrong, though, can be really tricky. (More so on Windows - troubleshooting on Windows often enough ends in "Let's just reboot and hope it works next time")
Great way to lay it out. How many hours have I spent debugging just to come to the conclusion that the code does everything according to the requirements — it's the requirements which are buggy.
I usually refactor after understanding the code specially oddball sections which usually means it's handling some non trivial corner case. Now there has been a fair amount of cases where a rotten code has survived and caused other bits of rotten codes to be added because everyone keeps thinking it must have a purpose when it really doesn't. It is a hard problem but sometimes you have to take a bit of risk specially if the current code/interface keep decreasing the quality of all the new codes surrounding it.
And it's one of the reason when reviewing code I am extra hard on code that solves problem in a complicated way or do extra pointless work since those can cause these type of confusion in future.
I actually did a fair amount of this at work in the latest release. It was a combination of business rule change, data structure change and since it was the core security model we also did major interface change (the one linus is mentioning) so that the rest of the app/developer can work on a simpler/easier model. I spend more days discussing and planning the change then the actual coding for once and despite the few bugs at QA it had been a success. It had been one of the biggest change in my 10 years of work and one of the most stable. My takeaway from that is to iterate over the design a lot then iterate over the code a lot before really pushing it everywhere and probably have some break in between (if it's important enough) to have multiple go at it with fresh perspective.
When I was almost finished with it, the one ugly element that remained was a C-style for loop with a head that filled one and a half lines (about 150-160 characters) of seriously cryptic code, and its body was not much better.
I had been explicitly ignoring this loop while I was working on the application, because I had no clue what it did, and the app was working more or less they way it should.
But when I was finished slightly ahead of schedule, I could not help but use the free time I had left trying to figure out what the for-loop did. Otherwise, the code I had been massaging for a couple of months now was pretty pretty, but the prettier it had become, the more this ugly for-loop stuck out.
I started by adding a simple logging statement to its body that simply announced the fact that the loop was executing an iteration. Then I played around with the application for a while. Eventually, I took a look at the log file to see how often the loop had executed, and was rather baffled - the loop had executed a total of zero times.
I played around with the app some more, trying my worst to make that loop run at least once (which was, admittedly, hard, because I had no clue what it was for). Two hours later, I gave up and removed the loop.
I finished my training about eight months later, and at least during that time, I did not hear of it again. (I really would have liked to know where that loop came from, though.)
That seems very risky, and I'd never have allowed that change to pass a code review. Just because you couldn't figure out how to make the loop execute doesn't mean that there aren't any paths through the code that would cause it to execute. The only safe way to remove it would be to prove that it can't ever execute -- e.g., the loop only executes if the variable foo is true (the condition is "foo && ..."), but foo is initialized to false and isn't modified anywhere else in the code.
Source: http://www.chesterton.org/taking-a-fence-down/
(The whole thing was less than a thousand lines of not-very-dense Perl code, plus a few hundred lines of HTML templates. It's not like this was some highly complex piece of code.)
What's wrong with strlcpy? When I need one, I take the code from OpenBSD: http://cvsweb.openbsd.org/cgi-bin/cvsweb/~checkout~/src/lib/...
Reasons for having it are surely not moronic because it provides security benefits.
I'm not sure what Linus intended.
https://www.sourceware.org/ml/libc-alpha/2000-08/msg00061.ht...
His issue was converting a whole lot of code over to strlcpy() while introducing bugs that didn't previously exist.
For another example of this kind of irrational thinking, look at NeoVim's massive churning changes to the Vim codebase for the purposes of "modernization".
I'm using Neovim currently and really enjoy that it supports stuff like 24 bit color schemes in the terminal. I finally can use color schemes which don't hurt my eyes.
We are generally very careful about "cleanup" patches in Neovim. Some examples:
https://github.com/neovim/neovim/pull/2418
https://github.com/neovim/neovim/pull/1788
https://github.com/neovim/neovim/pull/1340
At least one tangible benefit to some of these changes is that it allows us to enable -Wconversion.
We've also rejected dozens of cleanup patches for quality reasons.
EDIT: Or there is no NULL in src and it runs off into an unmapped page calculating the len :D Wow it's just a pretty bad idea for a lot of cases the more I think about it.
Not in e.g. Java and many scripting languages.
If your answer is "use a library", that holds for C too.
https://news.ycombinator.com/item?id=7191509
I've used Ustr before, all of them have pros and cons depending on what you are needing.
The volume of string libraries only suggests how crucial core library design, often an afterthought, is to programming language design. The language itself encourages idioms, and it's for the core library to harness these idioms into a robust, cohesive, comprehensive toolbox for common tasks.
My article about it:
http://digitalmars.com/articles/b44.html
foo(size_t n, char arr[restrict static n])
The static qualifier here means "no really, I actually want the compiler to complain if the passed array isn't the size indicated". (because C doesn't have enough different meanings for the word "static" :( ). I haven't seen this in the wild yet or checked if gcc and clang actually implement it usefully, but it seems like it could be quite useful.