> On Aug. 2, 2012, 2:58 p.m., Kenneth Giusti wrote: > > http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/CMakeLists.txt, > > line 39 > > <https://reviews.apache.org/r/6302/diff/1/?file=132723#file132723line39> > > > > Actually, I think driver.c should be split in two: there's quite a bit > > of platform generic code in this file that should be shared. > > > > Take a look at the "driver_abstraction" branch upstream. I've split > > driver.c into two files - isolating (some) of the platform code into a > > separate "impl" file. > > > > To support windows, we'd have to cleave off more code than I have in > > the branch, but it's a start.
I agree. The current windows/driver.c is a straw man, just to get us rolling. When the time comes to do it properly using I/O completion ports (and presumably stick handling the ssl layer too), I (or any interested party) can continue working with what you have. > On Aug. 2, 2012, 2:58 p.m., Kenneth Giusti wrote: > > http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/messenger.c, > > line 324 > > <https://reviews.apache.org/r/6302/diff/1/?file=132732#file132732line324> > > > > The sematics are somewhat different here: 0.0.0.0 == all addresses. Is > > this change ok? It is necessary to work on Windows, where the OS treats a connection to "all addresses" as meaningless, and returns an error. Listening on 0.0.0.0 is fine, and those uses are unchanged. > On Aug. 2, 2012, 2:58 p.m., Kenneth Giusti wrote: > > http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/proton-dump.c, > > line 35 > > <https://reviews.apache.org/r/6302/diff/1/?file=132733#file132733line35> > > > > Problem: calling sprintf may change errno, which means perror could > > print the wrong description. Good catch. will fix. > On Aug. 2, 2012, 2:58 p.m., Kenneth Giusti wrote: > > http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/driver.c, > > line 1 > > <https://reviews.apache.org/r/6302/diff/1/?file=132737#file132737line1> > > > > See my previous comment re: splitting up this file. > > > > I think we should come up with a layered abstraction here, so we can > > not only support different platforms (win vs unix), but also different i/o > > mechs (poll, epoll, windows completions), and the use of different TLS/SSL > > impls. Agreed. But I would like to get an initial Windows version going first. > On Aug. 2, 2012, 2:58 p.m., Kenneth Giusti wrote: > > http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/proton-dump.c, > > line 87 > > <https://reviews.apache.org/r/6302/diff/1/?file=132733#file132733line87> > > > > ditto ditto - Cliff ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/6302/#review9755 ----------------------------------------------------------- On Aug. 6, 2012, 8:16 p.m., Cliff Jansen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/6302/ > ----------------------------------------------------------- > > (Updated Aug. 6, 2012, 8:16 p.m.) > > > Review request for qpid, Andrew Stitcher, Kenneth Giusti, Steve Huston, and > Rafael Schloming. > > > Description > ------- > > This patch set works with a recent mingw32, cmake 2.8.1, python 2.5, swig > 2.0.7. > > A push-button build is still a ways off. The custom_commands in the cmake > script to generate the protocol headers don't work yet on Windows. > > The most intrusive change was the introduction of a pn_socket_t type to hold > a socket on both Windows and Posix platforms. An attempt was made to > minimize the use of #ifdefs and split platform code into separate posix and > windows directories, as is done for the C++ code. There is so little needed > at the moment, this may be overkill. The qpid-proton-posix library was > ditched and combined with the main qpid-proton library. Instead, the work is > done in CMake to assemble the correct shared and platform specific sources as > is done in the C++ tree. > > The driver.c implementation is proof of concept using Winsock select(). > Future work would most likely replace this with an I/O completion port > implementation. > > > 1. mkdir ...\trunk\proton-c\build > > 2. set env vars as per trunk\config.sh > > > set PATH=C:\Program Files (x86)\CMake > 2.8\bin;C:\python25;C:\mingw_ptn\bin;C:\Windows\System32;c:\cj\work\amqp\proton\mingw4\trunk\proton-c\build > set > PYTHONPATH=c:\cj\work\amqp\proton\mingw4\trunk\tests;c:\cj\work\amqp\proton\mingw4\trunk\proton-c;c:\cj\work\amqp\proton\mingw4\trunk\proton-c\build\bindings\python > set > PYTHON_BINDINGS=c:\cj\work\amqp\proton\mingw4\trunk\proton-c\build\bindings\python > set PROTON_HOME=C:\cj\work\amqp\proton\mingw4\trunk > > 3. generate the headers: > > cd trunk\proton-c\build > python > c:\cj\work\amqp\proton\mingw4\trunk\proton-c\src\codec\encodings.h.py > >encodings.h > python c:\cj\work\amqp\proton\mingw4\trunk\proton-c\src\protocol.h.py > >protocol.h > > 4. build > > cmake -G "MinGW Makefiles" -DSWIG_EXECUTABLE=C:\swigwin-2.0.7\swig.exe > c:\cj\work\amqp\proton\mingw4\trunk\proton-c > mingw32-make > python ..\..\tests\proton-test > > > This addresses bug QPID-4181. > https://issues.apache.org/jira/browse/QPID-4181 > > > Diffs > ----- > > http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/CMakeLists.txt > 1369190 > > http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/CMakeLists.txt > 1369190 > > http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/php/php.i > 1369190 > > http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/python/python.i > 1369190 > > http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/include/proton/driver.h > 1369190 > > http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/codec/codec.c > 1369190 > > http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/dispatcher/dispatcher.c > 1369190 > http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/driver.c > 1369190 > > http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/framing/framing.c > 1369190 > > http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/message/message.c > 1369190 > http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/messenger.c > 1369190 > > http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/proton-dump.c > 1369190 > http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/proton.c > 1369190 > > http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/network.h > PRE-CREATION > > http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/posix/time.c > PRE-CREATION > http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/uuid.h > PRE-CREATION > > http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/driver.c > PRE-CREATION > > http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/time.c > PRE-CREATION > > http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/uuid.c > PRE-CREATION > http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/util.h > 1369190 > http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/util.c > 1369190 > > http://svn.apache.org/repos/asf/qpid/proton/trunk/tests/proton_tests/messenger.py > 1369190 > > Diff: https://reviews.apache.org/r/6302/diff/ > > > Testing > ------- > > Fedora, Windows > > > Thanks, > > Cliff Jansen > >
