> On 2011-01-18 18:12:32, Nathan Binkert wrote:
> > src/base/vnc/vncserver.hh, line 150
> > <http://reviews.m5sim.org/r/432/diff/1/?file=9748#file9748line150>
> >
> >     Brace is usually on new line for struct, but in this case, perhaps we 
> > don't care.
> 
> Ali Saidi wrote:
>     There are so many of these i think it could go against Steve's desire for 
> dense code

Yeah, that's fine with me.  As long as there is no code inside, I'm cool.


> On 2011-01-18 18:12:32, Nathan Binkert wrote:
> > src/base/vnc/vncserver.hh, line 162
> > <http://reviews.m5sim.org/r/432/diff/1/?file=9748#file9748line162>
> >
> >     We generally try not to have __attribute__ stuff in our code, right?  
> > Don't we generally have a #define in src/base/compiler.hh and then use that 
> > here?
> >     
> >     Also, since you put the padding in, I'm not even sure that you need the 
> > __packed__
> 
> Ali Saidi wrote:
>     I added a define for it. We used to have defines that created ints with 
> certain alignments, but we got rid of them at some point. 
>     
>     I'd rather not try to debug an issue with the compiler randomly decides 
> to put some padding in the struck.

With a #define, I'm fine.  I don't know of any C compiler that doesn't align 
variables in a C struct according to their size (with a max alignment of 4 
bytes on 32-bit machines and 8bytes on 64-bit machines).  So, if we're not gcc, 
it will likely just work.  (I'd really like to use llvm some day).


> On 2011-01-18 18:12:32, Nathan Binkert wrote:
> > src/dev/ps2.cc, line 41
> > <http://reviews.m5sim.org/r/432/diff/1/?file=9761#file9761line41>
> >
> >     hmmm.  Should we put this in ext?  Otherwise, will we have a dependency 
> > on libx11-dev or something like that?
> 
> Ali Saidi wrote:
>     if you want. It's already been installed in every machine I've looked.

I do want.  ubuntu-server doesn't have any of them by default and I use that on 
most server machines.  Might be worthwhile to copy keysym.h into ext/, though I 
could live with the dependency if you want to use VNC.


- Nathan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/432/#review775
-----------------------------------------------------------


On 2011-01-18 14:49:34, Ali Saidi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/432/
> -----------------------------------------------------------
> 
> (Updated 2011-01-18 14:49:34)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> -------
> 
> VNC: Add vnc server to M5 and support to use it for ARM.
> 
> 
> Diffs
> -----
> 
>   src/dev/arm/pl111.hh 32b6354d2ae6 
>   src/dev/arm/kmi.cc 32b6354d2ae6 
>   src/dev/arm/amba_device.hh 32b6354d2ae6 
>   src/dev/arm/amba_device.cc 32b6354d2ae6 
>   src/dev/arm/kmi.hh 32b6354d2ae6 
>   src/dev/arm/RealView.py 32b6354d2ae6 
>   src/dev/SConscript 32b6354d2ae6 
>   src/base/vnc/vncserver.cc PRE-CREATION 
>   src/base/vnc/vncserver.hh PRE-CREATION 
>   src/base/vnc/convert.cc PRE-CREATION 
>   src/base/vnc/convert.hh PRE-CREATION 
>   src/base/vnc/VncServer.py PRE-CREATION 
>   src/base/vnc/SConscript PRE-CREATION 
>   configs/common/FSConfig.py 32b6354d2ae6 
>   src/base/SConscript 32b6354d2ae6 
>   src/base/bitmap.hh PRE-CREATION 
>   src/base/bitmap.cc PRE-CREATION 
>   src/dev/arm/pl111.cc 32b6354d2ae6 
>   src/dev/arm/rv_ctrl.hh 32b6354d2ae6 
>   src/dev/arm/rv_ctrl.cc 32b6354d2ae6 
>   src/dev/ps2.hh PRE-CREATION 
>   src/dev/ps2.cc PRE-CREATION 
>   src/sim/serialize.hh 32b6354d2ae6 
>   src/sim/serialize.cc 32b6354d2ae6 
> 
> Diff: http://reviews.m5sim.org/r/432/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ali
> 
>

_______________________________________________
m5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/m5-dev

Reply via email to