5 ms·

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(ite

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.

klodolph 5y ago
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.
- pitaj 5y ago
  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.
  - klodolph 5y ago
    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-clang-static-analysis-check.html 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.
    - andi999 5y ago
      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.
      - klodolph 5y ago
        “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.
      - plorkyeran 5y ago
        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.
  - karlding 5y ago
    You can also write your own standalone LibTooling [0] app, which accomplishes something similar [1]. [0] https://clang.llvm.org/docs/LibTooling.html https://clang.llvm.org/docs/LibTooling.html [1] https://clang.llvm.org/docs/LibASTMatchersTutorial.html https://clang.llvm.org/docs/LibASTMatchersTutorial.html
- slaymaker1907 5y ago
  Getting code analysis/refactoring tools to understand huge, complicated projects like the Linux kernel is far from trivial.
  - klodolph 5y ago
    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-extra/clang-tidy 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.
- jabl 5y ago
  Kernel developers have previously used a tool called coccinelle to do these kinds of mechanical mass changes. It's pretty nifty.

klodolph 5y ago

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.

pitaj 5y ago
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.
- klodolph 5y ago
  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-clang-static-analysis-check.html 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.
  - andi999 5y ago
    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.
    - klodolph 5y ago
      “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.
    - plorkyeran 5y ago
      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.
- karlding 5y ago
  You can also write your own standalone LibTooling [0] app, which accomplishes something similar [1]. [0] https://clang.llvm.org/docs/LibTooling.html https://clang.llvm.org/docs/LibTooling.html [1] https://clang.llvm.org/docs/LibASTMatchersTutorial.html https://clang.llvm.org/docs/LibASTMatchersTutorial.html
slaymaker1907 5y ago
Getting code analysis/refactoring tools to understand huge, complicated projects like the Linux kernel is far from trivial.
- klodolph 5y ago
  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-extra/clang-tidy 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.
jabl 5y ago
Kernel developers have previously used a tool called coccinelle to do these kinds of mechanical mass changes. It's pretty nifty.