----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3662/#review8927 -----------------------------------------------------------
src/sim/syscall_desc.hh (line 67) <http://reviews.gem5.org/r/3662/#comment7673> my preference would be to keep this as a one-liner as in the original code. I don't think expanding it makes it more readable. If anything, it makes the code seem more significant than it really is. src/sim/syscall_desc.hh (line 73) <http://reviews.gem5.org/r/3662/#comment7675> if you're going to create methods, might as well put the semantics here too. i.e., no need for separate warned() and setWarned() methods, just do something like: bool checkWarned() { bool was_warned = _warned; _warned = true; return was_warned; } and then the code in ignoreFunc() could be simplified a bit. I might even go further and, instead of checkWarned() like above, have a function like this: bool needWarning() { bool suppress_warning = warnOnce() && !_warned; _warned = true; return !suppress_warning; } and then you could rewrite ignoreFunc() along the lines of: if (desc->needWarning()) { warn("ignoring syscall %s(...)%s", desc->name(), desc->warnOnce() ? "\n (further warnings will be suppressed)" : ""); } src/sim/syscall_desc.hh (line 85) <http://reviews.gem5.org/r/3662/#comment7674> again, this could be a one-liner. - Steve Reinhardt On Oct. 17, 2016, 8:08 a.m., Brandon Potter wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3662/ > ----------------------------------------------------------- > > (Updated Oct. 17, 2016, 8:08 a.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11690:4bd82b7d3c09 > --------------------------- > syscall_emul: [patch 2/22] move SyscallDesc into its own .hh and .cc > > The class was crammed into syscall_emul.hh which has tons of forward > declarations and template definitions. To clean it up a bit, moved the > class into separate files and commented the class with doxygen style > comments. Also, provided some encapsulation by adding some accessors and > a mutator. > > The syscallreturn.hh file was renamed syscall_return.hh to make it consistent > with other similarly named files in the src/sim directory. > > The DPRINTF_SYSCALL macro was moved into its own header file with the > include the Base and Verbose flags as well. > > > Diffs > ----- > > src/arch/alpha/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/arm/freebsd/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/arm/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/mips/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/power/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/sparc/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/sparc/linux/syscalls.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/sparc/solaris/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/x86/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/x86/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/kern/tru64/tru64.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/SConscript 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/syscall_debug_macros.hh PRE-CREATION > src/sim/syscall_desc.hh PRE-CREATION > src/sim/syscall_desc.cc PRE-CREATION > src/sim/syscall_emul.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/syscall_emul.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/syscallreturn.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > > Diff: http://reviews.gem5.org/r/3662/diff/ > > > Testing > ------- > > > Thanks, > > Brandon Potter > > _______________________________________________ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
