Hacker News Reader

This is just how you work with large code bases. Updating 15,000 call sites is far from trivial. These changes happen in multiple phases, and in each phase, you still want to have a working kernel.

The unsafe version can be deprecated and eventually removed, but that is down the road.

I’m not sure how it’s done with the Linux Kernel (and git definately makes it hard), but at Google (using Perfoce model) there are tools for mass renaming that try to guarantee that there are no regressions. Of course in Linux the amount of C macros make it much harder to work on the syntax trees, but comparision after the preprocessor step should be possible.

IIRC Google works the same way. The automated refactoring tools (Rosie?) may make it go faster, but you generally don't fix up 15,000 call sites in a single change... you break it up into smaller batches spread across the tree.

It's been a long time since I was there, but I thought there were plenty of 15,000 call site refactorings done in a single final CL. Not that the Linux kernel should do the same!

When I was there (several years ago) it was rare to do that across many projects. You don't want to have to roll back everyone due to a problem that affects one project. Also, there's a risk that changes that are happening in the meantime might mean the patch doesn't apply.

For a global change? These days that 'risk' is more of a certainty.

It may take a lot of time to get all the approvals from all the code owners. One of the big advantages of a tool like Rosie is precisely that it splits up the update into smaller CLs and has automation that nags all necessary reviewers and reruns the transformation once head moves.

What does git or Perforce have to do with refactoring? Why would whatever version control you're using matter?

> tools for mass renaming that try to guarantee that there are no regressions

beyond sed or whatever a fancy IDE already does? What kind of voodoo magic are we talking about here? Did Google solve the halting problem and I just haven't noticed yet...

Disclaimer: I haven't worked at Google, but you can easily read their papers like https://dl.acm.org/doi/pdf/10.1145/2854146 and understand what's going on. Refactoring tooling is integrated with the revision control system because in a giant codebase with thousands of contributors, you need to split a refactoring patch into smaller chunks and ideally use semantic rather than text-based diff tooling, or the changes will never get reviewed or merged before they conflict with more changes.

As for the actual renaming, yes, definitely beyond what sed does but probably around what the fanciest IDE does. Imagine a big global symbol dependency graph produced by the entire build toolchain all at once and cached, I think?

Also, this isn't the halting problem unless your codebase allows for fully dynamic invocation :)

Aren't all function pointers dynamic invocation ? Or is "fully" one of those words that i'm not grasping in context.

> Aren't all function pointers dynamic invocation?

Given that compilers can be smart-enough to detect "non-dynamic" function-pointer invocation (e.g. when an execution-trace proves that a function-pointer parameter always points to the same function address), it's not safe to say that "all" function pointer invocations are dynamic.

Another case to consider is when one implements (Smalltalk-style) OOP with message-passing: in many cases it's possible to build that without needing to use function-pointers at all.

Also you have a type system. And even in C function pointers have types.

So you can exclude many things that would in-principle be possible.

> C function pointers have types.

I'm not 100% sure, but i remember that i could have have both a two and three function argument be called with the same function pointer, its probably UB however i think that as long as any dereferenced functions adhered to the standard argument behavior of the platform specific calling convention it worked, and i dont think gcc, clang or msvc complained, but my memory might be wrong.

The rename has to be atomic, which is one of the things your revision control system gives you, unless it's like CVS or something worse†. So, that's a necessary part of the solution space.

You want there to be two states: State A where the thing was called old_name and State B where the thing is called new_name. State AB where some code thinks it is called old_name but other code thinks it is called new_name is broken, and must not exist.

† This might seem outrageous to modern programmers, but CVS thinks in terms of files, so from its point of view it's fine if out of sixty files you tried to commit, 48 of them succeeded and 12 failed. Good luck fixing the resulting mess.

I remember having to go backwards from SVN to CVS for a project a long time ago, and I was so shaken by the fact that CVS couldn’t do atomic commits with multiple files!

Yeah, my favorite model of how git operates is "rewind all the way back to RCS, add atomic commits and take it from there".

SVN, baz/bzr and friends are what you get if you add networking before atomic commits. Git is what you get if you start with the proper data model and only then add networking.

I believe the commenter is referring to system where you take a giant “git sed” and automate a rather complicated process: 1. break up one huge diff into a set of many (hundreds or thousands) patches, where each individual patch only touches files in a particular subsystem. 2. send all those patches to the appropriate system owners and run the appropriate tests. 3. manage the actual merging of successful patches, as well as communicating to the original author any individual patches that might need more attention e.g. based on patch review feedback.

as you might imagine, it’s probably a pretty involved process to touch thousands of lines of code in a complicated system.

There are multiple branches being actively worked on. It would be very easy for someone to murge in an unrefacatored branch and now half the code base is using the old call.

You'd want something like https://bors.tech/ to avoid that.

Bors works for git. Google has similar tooling for their system. (I think it's called the 'train'.)

The point is that the change is not a simple rename:

    struct foo *iterator;

    list_for_each_entry(iterator, &foo_list, list) {
     do_something_with(iterator);
    }

While the new version would be something like:

    list_for_each_entry_v2(iterator, &foo_list, list) {
     do_something_with(iterator);
    }

So the ’iterator’ variable may be removed, but as this is c, might also be reused multiple times, or come from some struct member, union, or whatever. So i presume something like this would have to be fixed case by case.

You can do these kinds of changes with clang-tidy, if necessary. Clang-tidy is extensible and you can write your own rules for transforming source code.

Mind you, there are not a ton of people around who have the time to learn how to write custom clang-tidy rules. It might be done for 15,000 changes that potentially fix kernel vulnerabilities, though.

Unless something has changed in the last month, clang-tidy is not extensible. You can't write your own rules without forking the whole project.

I’ve worked at a couple companies that had custom clang-tidy checks. This is well-documented enough at this point, but it is still a bit arcane.

http://bbannier.github.io/blog/2015/05/02/Writing-a-basic-cl...

Yes, it involves forking. Forking isn’t that big a deal. We even had checks that were specific to internal libraries.

At the start forking is like no deal at all. But how do you get new clang tidy features from the main branch later into your fork; I think this is the only instance which I would really call technical debt.

“Git pull” works pretty well to bring new clang-tidy features downstream. If you write a custom check, it goes in its own set of files, so there will be no merge conflicts. The only thing that would break is when the internal clang-tidy API changes, but clang-tidy checks tend to be fairly short.

Really… I have worked at two companies that had extended clang-tidy to add custom checks for their code base. The whole point of clang-tidy is to automate code fixes in other parts of your code base. When done well, use of clang-tide pays down technical debt faster rather than adding to it.

Forking to add new checks only causes problems with pulling from upstream when APIs you are using are changed upstream, which happens rarely. Forking to _modify_ existing checks would cause you a lot of pain, but the solution there is to copy the existing check instead.

You can also write your own standalone LibTooling [0] app, which accomplishes something similar [1].

[0] https://clang.llvm.org/docs/LibTooling.html

[1] https://clang.llvm.org/docs/LibASTMatchersTutorial.html

Getting code analysis/refactoring tools to understand huge, complicated projects like the Linux kernel is far from trivial.

In practice it is not that hard. The refactoring tool may only need to tackle one translation unit at a time. You create a pattern which matches some usage of a macro, and create some logic to write out some changes to the AST. It can involve a surprisingly short amount of code—because clang-tidy has good tools for writing patterns and modifying ASTs!

You can brows the clang-tidy source code yourself. There are existing checks specific to the Linux kernel in there already. Well, there’s one check. But if you use clang-tidy, you’ll discover that automatic refactoring of extremely large code bases is within reach.

https://github.com/llvm/llvm-project/tree/main/clang-tools-e...

The only question is whether you would want to spend the staff hours working on a clang-tidy check. Large code bases are exactly where the tradeoff makes sense.

Kernel developers have previously used a tool called coccinelle to do these kinds of mechanical mass changes. It's pretty nifty.

Linux doesn't even have consistent testing let alone the kind of tooling or will that Google does.

The Linux kernel developers have pretty good tooling for refactoring. For example, they have Coccinelle and Sparse.

1. https://en.wikipedia.org/wiki/Coccinelle_(software) 2. https://en.wikipedia.org/wiki/Sparse

Linux definitely has testing. Some helpful person added our repo base to the kernel-test-robot list, and now I regularly get e-mails when something I'm working on in some random WIP branch broke the build on some other architecture (often in COMPILE_TEST mode), or some other random driver. Didn't even have to do anything myself. It's great for avoiding "this broke the build on $obscure_config" issues.

I don't see how Perforce makes this easier than Git?

I thought this was one of the main advantages of typed languages refactoring or renaming is safe and easy. What gives?

Atomically flipping a version is still difficult, as anything that needs to rely on the other version has to be fixed forward. If you instead make 5000 (or 500) small changes that affect 10-15 uses, you can solve the unique cases or roll-back and forward.

You can still have plenty of issues crop up from code that's not currently in master yet, or forked projects that take patches sparingly, etc.

Typed languages let you know whats broken at compile time and aides in refactoring code you can see. It doesn't help you refactor code you can't see.

While calling C a typed language is technically correct, I struggle to think of any typed language that's more weakly-typed than C. Arguably, Python has a stronger type system than C.

"static vs dynamic" and "strong vs weak" are orthogonal coordinates.

It seems GP was talking about the latter.

The C preprocessor is arguably a not type safe language unto itself.

The C preprocessor is a siren song, lures people in with the promise of "extra performance/features/structures for zero runtime cost!" but murders any readability or rationality within your codebase.

Cool concept, but once you've dealt with any legacy codebase that has used it extensively, it feels like an anti-pattern/footgun. Ultimately you just want the language to do those things directly and for the compiler to be smart enough to optimize it later.

Essentially it is too clever for its own good.

No one likes the preprocessor, but it's practicaly required if you don't want to move to C++ with template metaprogramming.

Well, C++ templates happen on the preprocessor too.

Not really? They're part of compilation and obey all the type rules & syntax of the language. They're not textual replacements that run in a distinctly different phase like macros are.

No they don't. Template instantiation happens during compilation, way later than the preprocessor.

where in hell did you learn that, this is entirely false

Well, it is today.

When I first learned C++, using cfront in the late 80's, lots of the language was implemented as C pre-processor macros.

maybe lots of the language, but not templates. Cfront 2 did not have templates, and Cfront 3 did have them without using the preprocessor.

You can even check out how it was done: https://github.com/seyko2/cfront-3/blob/master/src/template....

I do like the preprocessor and my major pet peeve with C is that the preprocessor has been stagnant for ages. Like, why the hell can't i do something like

    #define BASE hey
    #define HEADERS foo bar baz
    #append HEADERS bad bal bah
    #push OSSUFFIX
    #ifdef WIN32
    #define OSSUFFIX win32
    #else
    #define OSSUFFIX unknown
    #endif
    #foreach H HEADERS
    #eval include "$(BASE)/$(OSSUFFIX)/$(HEADERS)"
    #endfor
    #pop OSSUFFIX

People go to great lengths making all sorts of weird structures from x-macros, repeated statements, defines that only exist to be used by other defines, etc all to work around existing preprocessor limitations - and many of them would simply become unnecessary if the preprocessor could do things like variable editing, loops and being able to eval its own commands.

Even though some stuff can be done via language features, it is often necessary and more flexible to work with the source code itself.

One alternative is to just write a C code generator, which lets you mix C and some sensible language, eg. python. Then use that to generate the code which is then sent to gcc.

Its not too clever for its own good, its programmers that are too clever for C, and desperately need basic features, and its the only way to get them unless you switch language.

Its also surreally poorly designed, encouraging worse habits than C itself. Avoiding definition collisions and lack of namespaces alone make it horrific for any moderate sized project and up. Combine that with the near complete lack of static analysis tools, and its a recipe for disaster.

"arguably" is not needed. Preprocessor destroys most of the semantics and even some syntax of C.

Yeah, I do agree. Interestingly, you could argue that it's fairly strongly typed; at least according to some definitions (and btw "type-strength" is not a rigidly defined term, so the definition itself is somewhat debatable).

I don't claim to be an expert in knowing what is/isn't in the various standards; I just look at the build errors and static analysis alarms. That said, I think the argument is that in the vast majority of cases it's not actually possible to change the type of a variable; off the top of my head, I can only think of pointer co-erosion. Otherwise, if you define an unsigned int, it stays an unsigned int.

Now strong/weak typing isn't necessarily the same thing as type safety. C has always seemed astonishingly bad on that front. It's like they try to trick novice developers into thinking they have a robust type system sometimes.

If you define functions implicitly they will link to any symbol, EVEN A VARIABLE... this is bananas. I think I sort of understand why compilers work this way, but it really feels like a bug in the language. Under some compilers you might not even get a warning for implicit function, either! TI had a compiler that hid them by default.

Enums are garbage in C. Again, you can misuse them, and may not even get a warning. You can pass an enum for color to a function that takes an enum for kittens and the compiler will be happy as a clam. The way they're often used can cause constant implicit conversions every time there's an assign/compare. May not be a problem for most positive values, but it's annoying if you're trying to develop standard compliant code. MISRA defines an "essential type system" and normal enums usage violates it.

I'm probably forgetting a TON of deficiencies. Please add them or correct me where I'm wrong.

That’s a good question, it’s not fair that you’re being downvoted.

It’s the C preprocessor that causes a mess. Tooling for C and C++, like automatic refactoring, lags behind tooling for languages like Java, C#, and Go. Refactoring tools have to deal with macros, conditionals inside #if/#else blocks, and header search paths.

In this case, the refactoring involves removing a variable from the enclosing scope of a macro invocation. The most likely way to automatically refactor it would be to write a custom check in clang-tidy.

Whether something is (statically) typed or not is more of a continuum than a binary.

Eg C tracks whether a variable is an int or a char. Haskell also tracks whether a function causes side effects or not. More sophisticated systems also track whether a function has to return eventually or could run forever.

You can done it, without complex tooling:

1. Have 3 versions of the thing to change: select_nospec, select_nospec_safe, select_nospec_unsafe.

select_nospec_unsafe is identical to select_nospec. Do the mass replace. All stay same.

2. Delete select_nospec

3. Start migrating to select_nospec_safe. You can still do this in small steps, because you old thing stay.

4. Where select_nospec_unsafe must be, keep it and maybe mark with a comment or similar to not delete it later

5. You finish and all is changed.

Maybe replace select_nospec_safe to select_nospec if wanna.

When there are hundreds of branches, not just feature branches but real long-lived development branches, and pull requests are managed the way they are in the Linux kernel (chain of trust all the way up to Linus).. "Do the mass replace" becomes quite non-trivial I imagine.

This strategy handles that fine as long as none of those branches lasts from before step 2 until the optional step 6. You pull from a long-lived branch, get a compile error because it calls select_nospec, do the replacement at that callsite, and commit. A headache but a manageable one. If you want to backport new features to old stable kernel versions, you can include a patch supplying select_nospec_unsafe as a synonym for select_nospec.

The branches wont compile until you fix every instance though, so its not that risky. just a bit messy.

And if the original version is not deleted in step 2 but after the final, then nobody break!

Depends on who you want to put the burden on?

It's relatively easy to do the mass replace in master. You leave the burden of the replacement in the other branches to people who own those branches.

Set up an automated script, to keep the 'unreplaced' version out of master (and people can also use that script to check their own branches).

Not sure if that counts as non-trivial, yet?

There are probably some other trade-offs involved. The people running kernel development ain't idiots.

> Not sure if that counts as non-trivial, yet?

Assuming perfect tooling and infrastructure doesn't sound like it necessarily have to be a big deal.

But, how far into fantasy land are we? How much effort would be required to get there? Is this and all similar problems large enough to justify it?

Is adequately tooling and infrastructure even possible?

Does that make proposed changes, such as moving to C11, order of magnitudes more difficult?

I not arguing is effort-free, but is effort-minimal. The idea allows, at most, one or two major steps, the first and the biggest is as simple as is possible and can be done in one go, and have zero possibility of breaking.

The only major issues is that the change is let half-way. But that is only possible with MAJOR undisicpline.

---

The major point, to me, is that this EXCESSIVE fear of breakage is not good. Yes, making upgrades is pain, but you CAN make the pain tolerable with some planning.

Refactoring is like exercise for code: Everyone dislike the idea of excessive, but is GOOD.

And lets be honest, among all the things that could requiere a refactoring, this case is on the most simples of the simples scenario..

First, do not break.

Second, if it ain't broke, don't fix it.

Third, accept that when someone found your code is broken, they don't have to tell you.

lots of ways to do this. codemod, ratchets. It's a solved problem

I wish languages would make it easier to decouple the "name" from the "code".

The unison language offers an interesting take on the problem of code evolution backward compatibility. https://www.unisonweb.org/

> Same as the infamous PHP mysql_real_escape_string (don't make the mistake of using mysql_escape_string!) [2][3]

TBF that is really a straight bridge to the mysql C API, which is why it’s also in mysqli.

MySQL actually has a third one called mysql_real_escape_string_quote, because if the sql mode NO_BACKSLASH_ESCAPES is set the escaping function needs to know what context it’s used in, and thus what quote to double up, and mysql_real_escape_string will fail.

And of course, the real answer to wether you're supposed to use mysql_escape_string, mysql_real_escape_string or mysql_real_escape_string_quote is "why the fuck are you pasting queries together using string processing".

I get the point you’re trying to make, but the blame doesn’t really lay with php.

Like most things introduced in early php, the mysql extension was just wrapping c functions.

The concept of “real” escape comes from mysqls’ C API: https://dev.mysql.com/doc/c-api/5.6/en/mysql-real-escape-str...

That's the deal with backwards compatibility. Even third parties may depend on that name, it's not safe to go and change it.

On userspace that's the time when you deprecate an entire module and switch everything into a new name. The kernel does that once in a long while, but it's a bit harder for them.

The kernel doesn't care about third parties though, that's why the kernel API/ABI is not stable. This is one big advantage to doing it that way: sure, you need to have a consistent migration plan in flight to make sure you don't break mainline in the process, but you can immediately remove the old way without having to care about third parties, once you're done.

> Same as the infamous PHP mysql_real_escape_string (don't make the mistake of using mysql_escape_string!) [2][3] or whole host of C stdlib footguns like strcpy/strncpy

In all of those, the enhanced function takes more paramaters. Switching the existing name to require a new parameter will break existing code, and people will not want to update. Making the new parameter optional is possible, but IMHO is messier than a new function that has required parameters and then deprecating the old function (and eventually removing it).

You are making the rather optimistic assumption that here is a single definition of the macro. There are likely dozens, most of which share a name and randomly overwrite each other, others share the name but are subtly different. Macros are a truly terrible way to metaprogram. In my experience, if you spot one in a code base and can remove it, you have removed a bug you never knew you had, you spot one and change it, you have broken more things than you will ever know.