Messages in this thread | | | Date | Sat, 29 Aug 1998 11:52:56 +1000 | From | Richard Gooch <> | Subject | Re: 2.1.118 Tons of oopes |
| |
Doug Ledford writes:
Let me preface this my saying that Doug is the first person who has taken the trouble to respond to technical issues instead of dumb flames and personal insults. Thank you Doug for explaining rather than flaming.
> Richard Gooch wrote: > > > So what is the difference between breaking all source and then > > inserting NULLs, or appending the new method and taking advantage of > > automatic structure initialisation. The only difference I've seen is > > that one approach breaks things and requires lots of effort. > > Automatic structure initing is damn sloppy coding in the kernel. If > that's the way you want to do things, fine, but you won't get any > kudos from me for proper coding practice. Let's give a little > scenario.
This is used widely in the kernel. It certainly has precedent. What do you think of the use of the GCC extension where you only initialise those members you want? This is something Jakub pointed to, IIRC.
> Linus adds flush() to the struct. He puts it at the end, only the > NFS code uses it, every one else uses auto initing of their struct. > > Later he adds another new option to the end that not all fses need. > However, you decide you need it in your code. You add the > initializer to the end of your struct, but you forget that the flush > item was never added, so all of a sudden you now have a flush entry > that does something totally different and many people totally screw > their filesystems thanks to you.
What should happen when you make use of a new method is that you look at include/linux/fs.h for the structure declaration. That's what I do.
I also have a counter example to yours. Imagine a driver that implements open() and then check_media_change(). For the methods between these two and after check_media_change(), it places NULL. With the change made, the driver's check_media_change() gets inserted into the revalidate() method. So now we have silent breakage.
> Think it won't happen? It happened to me. While I was off working > on 2.0.x, the new SCSI code was added to 2.1.65. When I came back, > I didn't notice the changes to the scsi_host structure until about 4 > or 5 versions into 5.0.x driver. I then modified my struct > initializer so that I put the variable name with *every* > intitializer element to make sure I avoided this problem in the > future no matter what happened to the scsi_host structure.
So why didn't you look at the structure definition before patching?
> > Maybe the inner circle *does* know some deep dark secret, but *I* > > don't know it and it hasn't been made public. In light of this, my > > questions are entirely reasonable. Instead of flaming me for pursuing > > this, why not come forth with some clearer explanations? > > The clearer explanation is that Linus doesn't want to track down bugs caused > by someone else's sloppy, lazy coding. So, he forces the breakage in order > to make sure that all of the code is updated in the *right* way, not the > sloppy, break it in the future way of automatic initialization and > assumptions.
Thank you for being the first person to actually communicate this line of reasoning. From my example above, I hope it's clear that this change can also lead to silent breakage.
Now that this line of reasoning has been explained, I can see it's merits. I'm still not convinced that the level of breakage is better than the perceived benefits of forcing updates, but at least I can finally see where people are coming from.
Would it not be better to use that GCC extension for strucure initialisation that Jakub pointed out? That way, you would only need to define the members you care about, the others would be left as NULL. The critical difference would be that it is insensitive to the moving of members around.
Regards,
Richard....
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.altern.org/andrebalsa/doc/lkml-faq.html
| |