3 ms·

> Like that macro is just a crappy version of std::for_each. It really, truly is not comparable. list_for_each_entry does a well known operation that's just mo

> Like that macro is just a crappy version of std::for_each. It really, truly is not comparable. list_for_each_entry does a well known operation that's just moving some pointers around. Safe to do while holding a spinlock, or in interrupt context, etc. std::for_each is a generic iterator implementation. What code runs in order to iterate? Will it transparently take locks, risking a deadlock? Will it need to schedule()? You don't need to ask those questions about the standard linked list macro, and that has a lot of value when every instruction the compiler generates might matter.

kllrnohj 5y ago
list_for_each_entry just does blind string substitution. std::for_each is the basic same thing except with type safety & variable scope safety instead. It's still just substitutions. All your other stuff about for_each is just... Wrong? It's all well defined what it does. The substitutions that for_each makes and exactly what it calls are all defined. You could give it a container that does something stupid, but you could also give list_for_each_entry the wrong field or accidentally use it when it's no longer valid (literally the bug being fixed)
- brenns10 5y ago
  Point taken on how for_each works for linked lists. I'm (obviously) not a C++ expert and it shows in that context. I suppose the thrust of my opinion is that a lot of code depends on the fact that it iterates over a linked list, which is safe in many contexts. The idea that callers could substitute container types which break its assumptions seems unwelcome to me. Nobody provides the wrong fields to list_for_each_entry because they get very nasty compiler errors if they try. But anybody can plug a new container type into a for each statement and get new behavior that silently breaks the assumptions of the original code. I suppose that could be protected by a better type system though. There's a lot to improve on C even for kernel dev, and "C is greatest" isn't a hill I'm willing to die on :)
  - kllrnohj 5y ago
    > But anybody can plug a new container type into a for each statement and get new behavior that silently breaks the assumptions of the original code. How would it be "silent"? You're changing the container at the point where you're making any assumptions about it. This would just fall under normal code review purview, just like code reviews are how Linux enforces that anyone even used this linked list macro in the first place. C certainly doesn't care if you use list_for_each_entry for your container iteration, after all. But it'd be a lot easier to replace these linked lists with a vector that has the same observable contract if C++ was used, just would be way faster to iterate on.
midjji 5y ago
All for(auto x:xs){L} does is: { iter= xs.begin(); while(iter++ != xs.end()){L} } Not exactly hard to see what happens. In particular as iter is typically just a raw pointer. Regardless of if you use one or the other you must answer the question, Should I take locks, etc. With the macro you also have the concern that if any of the headers you include happens to change order of inclusions, the macro called may change without being noticeable. That is a harder problem.

kllrnohj 5y ago

list_for_each_entry just does blind string substitution. std::for_each is the basic same thing except with type safety & variable scope safety instead. It's still just substitutions. All your other stuff about for_each is just... Wrong? It's all well defined what it does. The substitutions that for_each makes and exactly what it calls are all defined. You could give it a container that does something stupid, but you could also give list_for_each_entry the wrong field or accidentally use it when it's no longer valid (literally the bug being fixed)

brenns10 5y ago
Point taken on how for_each works for linked lists. I'm (obviously) not a C++ expert and it shows in that context. I suppose the thrust of my opinion is that a lot of code depends on the fact that it iterates over a linked list, which is safe in many contexts. The idea that callers could substitute container types which break its assumptions seems unwelcome to me. Nobody provides the wrong fields to list_for_each_entry because they get very nasty compiler errors if they try. But anybody can plug a new container type into a for each statement and get new behavior that silently breaks the assumptions of the original code. I suppose that could be protected by a better type system though. There's a lot to improve on C even for kernel dev, and "C is greatest" isn't a hill I'm willing to die on :)
- kllrnohj 5y ago
  > But anybody can plug a new container type into a for each statement and get new behavior that silently breaks the assumptions of the original code. How would it be "silent"? You're changing the container at the point where you're making any assumptions about it. This would just fall under normal code review purview, just like code reviews are how Linux enforces that anyone even used this linked list macro in the first place. C certainly doesn't care if you use list_for_each_entry for your container iteration, after all. But it'd be a lot easier to replace these linked lists with a vector that has the same observable contract if C++ was used, just would be way faster to iterate on.

midjji 5y ago

All for(auto x:xs){L} does is: { iter= xs.begin(); while(iter++ != xs.end()){L} } Not exactly hard to see what happens. In particular as iter is typically just a raw pointer. Regardless of if you use one or the other you must answer the question, Should I take locks, etc. With the macro you also have the concern that if any of the headers you include happens to change order of inclusions, the macro called may change without being noticeable. That is a harder problem.