On 04/27/12 11:23, Gabe Black wrote: > On 04/27/12 11:12, Steve Reinhardt wrote: >> On Fri, Apr 27, 2012 at 10:55 AM, Gabe Black <[email protected]> wrote: >> >>> If I recall, there were real problems with at least some of those >>> patches. The last time we brought them up we acknowledged at least a >>> problem with the ioctl patch(es) but I don't think they were ever >>> addressed. >>> >> The problems have not been addressed, no. How significant they are is a >> matter of opinion. Ideally someone would step up and address them and we'd >> be done with it. I'm saying that if no one is going to do that, we should >> consider just committing them anyway, with comments indicating what needs >> to be fixed, rather than forcing users to work around their absence for >> years to come. >> >> Steve >> _______________________________________________ >> gem5-dev mailing list >> [email protected] >> http://m5sim.org/mailman/listinfo/gem5-dev > I think I'd been waiting for Vince Weaver to fix them. I'll take a look > this weekend if I get a chance. > > Gabe > _______________________________________________ > gem5-dev mailing list > [email protected] > http://m5sim.org/mailman/listinfo/gem5-dev
Now that I've looked at these again, some probably can be committed (which I just did), some should be fixed up but are basically ok (the haddps and the x87 inst patch) and some should not be committed (the ioctl and rlimit ones). Also looking at the reviews, this a matter of *my* opinion, because I was primarily the one that pointed out the flaws in these patches. I don't want to be in the position where somebody does something part of the way, it makes it onto review board, and then either I get stuck doing it the rest of the way or I just concede and let something half done get into the repository. I don't appreciate the insinuation that my comments are insignificant (because their significance is a matter of opinion) and that these patches should be committed anyway just because no one has found the time to do the right thing and fix them. I have just as much of an obligation to take up sailboating or stamp collecting as I do to fix these patches. As far as the ioctl patch, I suspect the reason Vince Weaver wanted to nuke the cases present in ioctlFunc is that the constants they rely on don't exist in x86. They're defined for Alpha, MIPS, SPARC and powerpc, and in the file that uses them in the kernel (drivers/tty/tty_ioctl.c) conditionally compiles in the code that implements those calls based on if the constant is defined. Attaching the ioctl functions breaks the build because those constants aren't defined, and they aren't defined because there's nothing to set them to. Gabe _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
