> On Sept. 28, 2015, 8:39 a.m., Andreas Hansson wrote: > > src/dev/x86/i82094aa.hh, line 108 > > <http://reviews.gem5.org/r/3117/diff/2/?file=50022#file50022line108> > > > > override > > Joel Hestness wrote: > Can you clarify? This declaration overrides the virtual declaration in > IntDevice, and I don't expect anyone will want to inherit from I82094AA. Am I > missing something? > > Andreas Hansson wrote: > If it is overriding the virtual method in the base class we should add > the keyword "override" (or M5_ATTR_OVERRIDE, although the latter is no longer > needed). > > From clang 3.5 the compiler even enforces it when using -Wall. > > Joel Hestness wrote: > Ok, for consistency, I've added M5_ATTR_OVERRIDE, since there are no uses > of 'override'. > > Would you mind adding some notes about this to the wiki? Currently, it's > not clear from existing code how this needs to be used. There are no uses of > 'override', and use of 'M5_ATTR_OVERRIDE' is pretty spotty (only consistent > on drain, checkpointing). > > Andreas Hansson wrote: > Thanks. We'll sort out the consistency in an upcoming patch. The version > of clang that ships with XCode is not happy without it.
Cool. Is this patch alright to ship? - Joel ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3117/#review7296 ----------------------------------------------------------- On Sept. 27, 2015, 7:52 p.m., Joel Hestness wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3117/ > ----------------------------------------------------------- > > (Updated Sept. 27, 2015, 7:52 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11145:b92a7a451890 > --------------------------- > arch, x86: Delete packet in IntDevice::recvResponse > > IntDevice::recvResponse is called from two places in current mainline: (1) the > short circuit path of X86ISA::IntDevice::IntMasterPort::sendMessage for atomic > mode, and (2) the full request->response path to and from the x86 interrupts > device (finally called from MessageMasterPort::recvTimingResp). In the former > case, the packet was deleted correctly, but in the latter case, the packet and > request leak. To fix the leak, move request and packet deletion into IntDevice > inherited class implementations of recvResponse. > > > Diffs > ----- > > src/dev/x86/i82094aa.hh bd894d2bdd7c > src/dev/x86/i82094aa.cc bd894d2bdd7c > src/dev/x86/intdev.hh bd894d2bdd7c > src/dev/x86/intdev.cc bd894d2bdd7c > > Diff: http://reviews.gem5.org/r/3117/diff/ > > > Testing > ------- > > Compiled gem5.debug with --without-tcmalloc. Ran x86 FS tests with Valgrind. > > > Thanks, > > Joel Hestness > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
