Re: [Barry-devel] WinCE port of Barry
On Wed, Oct 10, 2012 at 05:32:36PM +0100, Toby Gray wrote: Good point. I've put a fix for this in my wince branch. Hi Toby, Sorry for the delay. I've merged your wince branch, all 3 changes that I found there. Thanks! - Chris -- Everyone hates slow websites. So do we. Make your web apps faster with AppDynamics Download AppDynamics Lite for free today: http://p.sf.net/sfu/appdyn_d2d_nov ___ Barry-devel mailing list Barry-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/barry-devel
Re: [Barry-devel] WinCE port of Barry
On 05/10/12 02:35, Chris Frey wrote:
On Thu, Sep 27, 2012 at 04:55:19PM +0100, Toby Gray wrote:
Hi,
I've finally managed to get enough spare timeto make the changes that
Chris suggested in his review comments. I've updated my wince branch in
github:
https://github.com/tobygray/barry/tree/wince
Thanks Toby! My time for Barry has been limited lately, but I took a
brief look tonight.
That's understandable. I know how easy it is for other work to get in
the way.
The good news is that I managed to get a solution to the tr1 problem
without needing to change the barry headers:
https://github.com/tobygray/barry/commit/af3d23b39279930f755bfb8a0aac5d91e5f7aec3
Nice.
Regarding the new TCP stream support:
I notice that in the unix implementation for brawchannel, you use both
::read() and fwrite() in the Stdin/Stdout stream classes. Is there a
reason for this? The ::read() is straight, unix, unbuffered I/O, while
the fwrite() is buffered. For buffered streams, we normally use cout.
It is safe to use fwrite() here, since we have cout.sync_with_stdio(true);
in most of the tools as I recall, but it would be nice if Barry code
didn't have this dependency.
I think ::write() is satisfactory, unless you know something I don't. :-)
Good point. I've put a fix for this in my wince branch.
I've merged this, since this is strictly tools/ code, but I can see
stream classes like this being useful elsewhere, and could in the future
be moved to a library, so it would be good to change this. Also...
+TcpStream::TcpStream(const char * addr, long port)
+{
+ mImpl.reset(new TcpStreamImpl);
+ mImpl-mListenAddress = addr;
+ mImpl-mPort = port;
+ mImpl-mSocket = INVALID_SOCKET;
+ mImpl-mListenSocket = socket(PF_INET, SOCK_STREAM, IPPROTO_TCP);
+ if( mImpl-mListenSocket == INVALID_SOCKET ) {
+ cerr Failed to create listening socket:
+ errno endl;
+ }
+ if( mImpl-mListenAddress == NULL ) {
+ mImpl-mHostAddress.s_addr = INADDR_ANY;
+ } else {
+ mImpl-mHostAddress.s_addr = inet_addr(mImpl-mListenAddress);
+ }
+}
This takes an external const char* and saves it internally, for later use.
This is also dangerous in code that could be used in the library. I'd use
a std::string in the TcpStreamImpl struct here for safety.
Another good point. Again I've put a fix for this in my wince branch.
I also fixed the registering for data packet handling without needing
the messy transfer interest function that my previous patches had
implemented.
This is better than TransferInterest(). Clever too. I didn't like exposing
RegisterInterest() with a type argument as the default option, so I
modified things a bit and added comments, trying to make it clear that
most users shouldn't use the new API. Socket::SyncSend() really needs
both data and sequence packets in order to function, so this entire API
is really only for the socket opening race condition. It still seems
a bit heavy to me, but I don't have a better way in mind, and this is
pretty clean as is! :-) Thanks!
It's a shame it is a bit complex, but at least it avoids the
TransferInterest and redelivering packets. I imagine that this hasn't
been seen before as none of the other socket mode protocols start with
data coming from the device.
I based all these changes off the v0.19.0 branch rather than master as I
that seemed to make more sense.
I've rebased the v0.19.0 branch onto master, and have included all your
work.
Thanks again!
Thank you for reviewing and pulling my changes.
Regards,
Toby
--
Don't let slow site performance ruin your business. Deploy New Relic APM
Deploy New Relic app performance management and know exactly
what is happening inside your Ruby, Python, PHP, Java, and .NET app
Try New Relic at no cost today and get our sweet Data Nerd shirt too!
http://p.sf.net/sfu/newrelic-dev2dev
___
Barry-devel mailing list
Barry-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/barry-devel
Re: [Barry-devel] WinCE port of Barry
Hi, I've finally managed to get enough spare timeto make the changes that Chris suggested in his review comments. I've updated my wince branch in github: https://github.com/tobygray/barry/tree/wince The good news is that I managed to get a solution to the tr1 problem without needing to change the barry headers: https://github.com/tobygray/barry/commit/af3d23b39279930f755bfb8a0aac5d91e5f7aec3 I also fixed the registering for data packet handling without needing the messy transfer interest function that my previous patches had implemented. I based all these changes off the v0.19.0 branch rather than master as I that seemed to make more sense. Regards, Toby On 16/07/12 12:10, 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 I don't think it's currently in a state to merge back into barry as I've made a few changes which I'm not entirely sure about if they are the best thing to do. 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 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 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? I noticed a couple of issues with routing of data and sequence packets early on in channel setup if the device sends data to the PC quickly. I fixed these in https://github.com/tobygray/barry/commit/ebbdf4c9d5868a69d636119da354547918c70e1a and https://github.com/tobygray/barry/commit/6dff36f6c2cd1ef2c0006af2baebbf5ee0c0c291 The rest of the commits are a combination of changes to the code to fix warnings or errors from the Microsoft compiler. If the commit messages aren't detailed enough for any of these then let me know and I'll fill in the gaps (and update the commit messages). Regards, Toby -- Live Security Virtual Conference Exclusive live event will cover all the ways today's security and threat landscape has changed and how IT managers can respond. Discussions will include endpoint security, mobile security and the latest in malware threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/ ___ Barry-devel mailing list Barry-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/barry-devel -- Everyone hates slow websites. So do we. Make your web apps faster with AppDynamics Download AppDynamics Lite for free today: http://ad.doubleclick.net/clk;258768047;13503038;j? http://info.appdynamics.com/FreeJavaPerformanceDownload.html___ Barry-devel mailing list Barry-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/barry-devel
Re: [Barry-devel] WinCE port of Barry
On Mon, Jul 16, 2012 at 12:10:28PM +0100, Toby Gray wrote: I noticed a couple of issues with routing of data and sequence packets early on in channel setup if the device sends data to the PC quickly. I fixed these in https://github.com/tobygray/barry/commit/ebbdf4c9d5868a69d636119da354547918c70e1a and https://github.com/tobygray/barry/commit/6dff36f6c2cd1ef2c0006af2baebbf5ee0c0c291 I'm not overly keen on the TransferInterest() function. For one it requires the non-scoped version of the scope_lock class, which I haven't merged. And for another, it adds a new call point for callbacks. Ideally, callbacks should occur from the same thread that calls DoRead, whatever application thread is doing that main processing. I don't think it is guaranteed that the worker thread will call TransferInterest(). In fact, I think it is likely that it won't. Is there a way to avoid TransferInterest completely? The application is in control of its read thread, and so is the raw channel code, if I'm not mistaken. Is there no way to drain the incoming queue before we transfer interest to a callback? Thanks, - Chris P.S. I haven't heard much from you in response to my queries. If you're just busy, that's ok, but if you're waiting for something from me, please do let me know. :-) I've merged all I can so far. -- Live Security Virtual Conference Exclusive live event will cover all the ways today's security and threat landscape has changed and how IT managers can respond. Discussions will include endpoint security, mobile security and the latest in malware threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/ ___ Barry-devel mailing list Barry-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/barry-devel
Re: [Barry-devel] WinCE port of Barry
On 25/07/12 07:25, Chris Frey wrote: On Mon, Jul 16, 2012 at 12:10:28PM +0100, Toby Gray wrote: I noticed a couple of issues with routing of data and sequence packets early on in channel setup if the device sends data to the PC quickly. I fixed these in https://github.com/tobygray/barry/commit/ebbdf4c9d5868a69d636119da354547918c70e1a and https://github.com/tobygray/barry/commit/6dff36f6c2cd1ef2c0006af2baebbf5ee0c0c291 I'm not overly keen on the TransferInterest() function. For one it requires the non-scoped version of the scope_lock class, which I haven't merged. And for another, it adds a new call point for callbacks. Both of those are good points. Ideally, callbacks should occur from the same thread that calls DoRead, whatever application thread is doing that main processing. I don't think it is guaranteed that the worker thread will call TransferInterest(). In fact, I think it is likely that it won't. That's also a very good point. Is there a way to avoid TransferInterest completely? The application is in control of its read thread, and so is the raw channel code, if I'm not mistaken. Is there no way to drain the incoming queue before we transfer interest to a callback? I did start by trying to do something similar but couldn't work out how to avoid the race between draining the incoming queue and registering the callback interest. If the registering of the callback is done first then it's possible for callbacks for new data to occur before the incoming queue is flushed. I think the alternative would be to support registering the callback before the socket is opened so that the packets can be routed correctly by the read thread while the thread opening the socket is still inside SocketZero::Open(). Unless I hear otherwise, I'll have a go at doing it this way and send an email to the list once I get some spare time. P.S. I haven't heard much from you in response to my queries. If you're just busy, that's ok, but if you're waiting for something from me, please do let me know. :-) I've merged all I can so far. Sorry about taking so long to respond, I'm not waiting for anything from you, I'm just busy. I wanted to respond to your comments with suggested code fixes, but just as I think I've got time to do that something else turns up. Regards, Toby -- Live Security Virtual Conference Exclusive live event will cover all the ways today's security and threat landscape has changed and how IT managers can respond. Discussions will include endpoint security, mobile security and the latest in malware threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/ ___ Barry-devel mailing list Barry-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/barry-devel
Re: [Barry-devel] WinCE port of Barry
On Mon, Jul 16, 2012 at 12:10:28PM +0100, Toby Gray wrote:
The rest of the commits are a combination of changes to the code to fix
warnings or errors from the Microsoft compiler. If the commit messages
aren't detailed enough for any of these then let me know and I'll fill
in the gaps (and update the commit messages).
I'm merging in the obvious fixes into master, and the changes that I think
might need to wait until version 0.19.0 (due to abi changes, etc), I've put
in a new branch v0.19.0. Expect that branch to be rebased on master
periodically. I'm not quite done yet.
You should be able to rebase your branch onto the v0.19.0 branch to see
what I haven't processed yet.
Some questions:
1)
Can you explain why you need a lock() member in scoped_lock?
The idea behind scoped_lock is to force its use in a scoped section,
to prevent forgotten unlocks and to make the sections obvious.
Was there a real need for it?
2)
Author: Toby Gray toby.g...@realvnc.com
Date: Thu Jun 7 15:46:53 2012 +0100
lib: Fixing incorrect forward declaration of ProbeResult as a struct, it's r
eally a class.
diff --git a/src/controller.h b/src/controller.h
index 8e3ba85..41727c7 100644
--- a/src/controller.h
+++ b/src/controller.h
@@ -32,7 +32,7 @@ namespace Barry {
// forward declarations
class SocketRoutingQueue;
-class ProbeResult;
+struct ProbeResult;
class PrivateControllerData;
Is this a problem with the Microsoft compiler? I believe that strictly
speaking, according to the C++ spec, class and struct can be used
interchangeably.
Thanks,
- Chris
--
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
___
Barry-devel mailing list
Barry-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/barry-devel
