> On Dec. 3, 2012, 8:01 p.m., Cliff Jansen wrote: > > The changes in platform.h and platform.c will be multiplied for mingw and > > Visual Studio. In https://reviews.apache.org/r/6302, a separate sys > > directory was proposed with uuid.h and time.h (and eventually driver > > components). Instead of ifdefs, the CMakeLists.txt file would pull in the > > appropriate sys/XXX/time.c file. This is more the way it is done in the > > qpid cpp tree. > > > > The volume of these changes (including going forward) is probably much less > > than in the qpid/cpp tree, so I do not foresee unamanageble nested ifdef > > hell in this approach. Since you are more ready for a commit, I can adjust > > if the chosen path is to go the ifdef route. > > Andrew Stitcher wrote: > I'm not at all opposed to introducing a more thorough change than this > and I'd even say that this change could be a stepping stone to that, however > for the present this was simple enough to cover nearly all the Unix bases. > > If adding windows adds a lot more complexity then by all means add a > platform directory instead of a simple file with different implementations. > [I prefer the terminology platform because we used sys before and it was > sufficiently ambiguous to not help keep the platform only aspects isolated > there]. > > Andrew Stitcher wrote: > As an aside I'd say the point to introduce extra files is when the > #ifdefs start getting nested as you indicate in your comment.
Then I say "Ship it!" I should have the mingw availabe real soon now. The review for that can revisit the ifdef question if someone voices an alternate preference. +1 on "platform" versus "sys". - Cliff ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8321/#review13983 ----------------------------------------------------------- On Dec. 3, 2012, 8:16 p.m., Andrew Stitcher wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/8321/ > ----------------------------------------------------------- > > (Updated Dec. 3, 2012, 8:16 p.m.) > > > Review request for qpid. > > > Description > ------- > > These changes move in the direction of abstracting away some portability > problems with the current proton code base: > - Detection of correct API for uuids; > - Detection of correct API for clock_gettime()/gettimeofday() > - Handling ignoring SIGPIPE on writing to closed sockets > - Allowing warnings to be turned of whilst building (but having them on is > still the default) > > You can also see (and comment) on these changes at github: > https://github.com/astitcher/qpid-proton/commits/portability > > > This addresses bugs proton-104, proton-105, proton-106, and proton-168. > https://issues.apache.org/jira/browse/proton-104 > https://issues.apache.org/jira/browse/proton-105 > https://issues.apache.org/jira/browse/proton-106 > https://issues.apache.org/jira/browse/proton-168 > > > Diffs > ----- > > /proton/trunk/proton-c/CMakeLists.txt 1415142 > /proton/trunk/proton-c/include/proton/driver.h 1415142 > /proton/trunk/proton-c/src/driver.c 1415142 > /proton/trunk/proton-c/src/messenger.c 1415142 > /proton/trunk/proton-c/src/platform.h PRE-CREATION > /proton/trunk/proton-c/src/platform.c PRE-CREATION > > Diff: https://reviews.apache.org/r/8321/diff/ > > > Testing > ------- > > Compiles and passes the proton-test suite on both Fedora 17 and FreeBSD > > > Thanks, > > Andrew Stitcher > >
