Re: [Barry-devel] WinCE port of Barry

[Chris F. <cd...@fo...>]

On Mon, Jul 16, 2012 at 12:10:28PM +0100, Toby Gray wrote:
> Hi,
> 
> I've been working on a port of Barry to WinCE. It's currently sitting in 
> the "wince" branch on my github:
> https://github.com/tobygray/barry/tree/wince

Thanks Toby!  Looks like a fair bit of code... I'll probably have more
comments after I review it more closely.


> The main commit which adds WinCE support is 
> https://github.com/tobygray/barry/commit/d660a40f21b823510e5b81113f20fe4af3c3974a. 
> This adds a wince directory in the root to contain the build files and 
> implementations of some functions not present on WinCE, such as getopt and 
> sleep. I have a feeling that these headers and source code should probably 
> be moved into /src, to go near pre-existing support code such as 
> getpwuidandroid.cc. Would that make sense?
> 
> I have already done similarily for configfile.cc, splitting it up into 
> common code, code for unix systems and code for win32 systems in 
> https://github.com/tobygray/barry/commit/36131b090c3d6c1c34af441b40868b3b094fa66b

With all the headers in wince/, I'm tempted to try to keep wince-specific
code in its own directory, since it will likely be larger than a simple
file here or there like android.

None of Barry is specifically written with '/' vs. '\' in mind, so you
kind of have your work cut out for you, in those spots. :-)  Fortunately,
most of that code is in the GUI code, I think.  I would sincerely like to
keep such things as low level and out of the way as possible, so I like
what you've done with configfile.cc so far.

What do you think of creating a static library in wince/ which is linked
to the main library when building for Windows?  Maybe that's what you're
doing already.


> As WinCE doesn't have tr1 support I had to change how the shared_ptr 
> support in Barry was referenced. Instead of directly referencing 
> tr1::shared_ptr, I changes this so that there is Barry::tr1::shared_ptr 
> which is provided by boost::shared_ptr on WinCE and by tr1::shared_ptr 
> on all pre-existing platforms.
> 
> This was done in 
> https://github.com/tobygray/barry/commit/b4a70d9f7feedaa52bebe46d78e7dce880e73b74. 
> I'm not entirely happy with this, especially as it changes the external API 
> for libbarry, but I couldn't think of an alternative solution.

I think it's better to go in the other direction.  For systems that don't
have tr1, then:

namespace std {
	namespace tr1 {
		using boost::shared_ptr;
	}
}

I hope that's possible. :-)  It might even prevent the need to bump to 0.19.


> I added support for brawchannel to read and write the channel data over 
> TCP, instead of STDIN and STDOUT as WinCE doesn't have great terminal 
> support. This has made tools/brawchannel.cc a bit of a mess of #ifdef 
> WIN32. I think the best thing to do is to split it into 
> tools/brawchannel.cc, tools/brawchannel_unix.cc and 
> tools/brawchannel_win32.cc. Does that make sense?

Yes, avoid #ifdefs if at all possible, please. :-)


More comments as I read through the code.

Thanks!
- Chris