----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/631/#review1114 -----------------------------------------------------------
Can you separate the output directory stuff for the next version of the diff. Overall, I'm ok with this code, though I'd rather see us move it into python some day. Python would certainly make it a lot simpler. src/base/output.hh <http://reviews.m5sim.org/r/631/#comment1517> heh, pedantic. I guess we may some day run on another OS. :) src/base/output.hh <http://reviews.m5sim.org/r/631/#comment1514> You should get rid of this, use the const version you added below everywhere and require the user to call create if find returns null. Requires changes elsewhere though. Create on side effect was a bad idea. src/base/output.cc <http://reviews.m5sim.org/r/631/#comment1515> Please use: if (i->second != openStream) continue; and remove a level of indent. I know this isn't in the style guide, but it definitely leads to more readable code. src/base/output.cc <http://reviews.m5sim.org/r/631/#comment1516> What's the point of the else block if the if (fs) has a "break;" in it? src/base/output.cc <http://reviews.m5sim.org/r/631/#comment1518> If you choose not to get rid of this version of the function, it should at least be based on the const version and not duplicate code. src/base/output.cc <http://reviews.m5sim.org/r/631/#comment1519> Wondering if we should give these more standard names like "mkdir" src/base/output.cc <http://reviews.m5sim.org/r/631/#comment1520> Could this happen? Would it resolve? Shouldn't we prevent this on the creation side or in resolve itself? - Nathan On 2011-04-10 16:50:13, Ali Saidi wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/631/ > ----------------------------------------------------------- > > (Updated 2011-04-10 16:50:13) > > > Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and > Nathan Binkert. > > > Summary > ------- > > VNC: Add support for capturing frame buffer to file each time it is changed. > > When a change in the frame buffer from the VNC server is detected, the new > frame is stored out to the m5out/frames_*/ directory. Specifiy the flag > "--frame-capture" when running configs/example/fs.py to enable this behavior. > > > Diffs > ----- > > configs/example/fs.py 02cb69e5cfeb > src/base/bitmap.hh 02cb69e5cfeb > src/base/bitmap.cc 02cb69e5cfeb > src/base/output.hh 02cb69e5cfeb > src/base/output.cc 02cb69e5cfeb > src/base/vnc/VncServer.py 02cb69e5cfeb > src/base/vnc/convert.hh 02cb69e5cfeb > src/base/vnc/convert.cc 02cb69e5cfeb > src/base/vnc/vncserver.hh 02cb69e5cfeb > src/base/vnc/vncserver.cc 02cb69e5cfeb > > Diff: http://reviews.m5sim.org/r/631/diff > > > Testing > ------- > > > Thanks, > > Ali > > _______________________________________________ m5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/m5-dev
