----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/839/#review1498 -----------------------------------------------------------
Looking at this now, I'd say the #1 problem is with the function name checkAndAllocNextPage, which manages to be underinformative despite being long and unwieldy. If we worked the term "stack" into that function name somewhere it would help a lot. How about changing it to something like "tryStackAllocation()"? Also, since Ali mentioned doxygen comments, I checked the comment for this function in process.hh and not only is it not doxygen formatted, but it's plain misleading (it should just say that the return value indicates success, not that we're about to panic). Seems like this would be a good time to fix that too. - Steve On 2011-09-05 18:54:18, Gabe Black wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/839/ > ----------------------------------------------------------- > > (Updated 2011-09-05 18:54:18) > > > Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and > Nathan Binkert. > > > Summary > ------- > > Stack: Tidy up some comments, a warning, and make stack extension consistent. > > Do some minor cleanup of some recently added comments, a warning, and change > other instances of stack extension to be like what's now being done for x86. > > > Diffs > ----- > > src/arch/alpha/faults.cc b3585da1f970 > src/arch/sparc/faults.cc b3585da1f970 > src/arch/x86/tlb.cc b3585da1f970 > src/sim/process.cc b3585da1f970 > > Diff: http://reviews.m5sim.org/r/839/diff > > > Testing > ------- > > > Thanks, > > Gabe > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
