Hi Manuel, > Unfortunately, I didn't have too much time to devote to this activity > later. Nevertheless, I was able to fix the last bug > preventing the tests > to pass completely on my solaris build. The remaining problem was > submitted as QPID-1183.
Congratulations! You win the first completed port prize :-) I have no authority to say how to do the rest of the integration, but I have some comments based on the ongoing Windows work and past experience with porting a codebase to many platforms. > 1.- Missing include files. I think this shouldn't hurt the > linux build, > and I think that these changes could be commited in a single patch. Probably you're right. > 2.- Some GNUishms in system calls. I think there're two cases > for this: > 2.1.- POSIX strerror_r doesn't return the buffer. > Temporarily, I've > used a #ifndef clause, but I think that using the POSIX way > everywhere > should be better: > > #ifndef SUNOS > return std::string(strerror_r(err, buf, sizeof(buf))); > #else > //POSIX strerror_r doesn't return the buffer > strerror_r(err, buf, sizeof(buf)); > return std::string(buf); In the Windows port, this function ends up in its own file qpid/sys/posix/StrError.cpp (because Windows requires a different implementation all together). You could put yours in qpid/sys/solaris/StrError.cpp after the Windows patches with the split, or do the split now in your workspace. > 2.2. - pthread_yield is not POSIX compliant. Using sched_yield for > the Solaris version, with a #ifdef/#ifndef clause. Right... You could copy the Thread implementation to solaris and reimplement with a different yield... This is the sort of approach Andrew urged me to follow with Windows - copy and change/reimplement rather than add ifdefs. > 3.- Solaris doesn't define the FTP and LOG_FTP syslog > categories. Fixed with a #ifdef/#ifndef clause. Right... I am looking at this area today wrt Windows since Windows has none of this stuff and I fear it's creeped too far into platform-agnostic code. I'll send another email on this later. > 4.- The private inheritance bug in the solaris compiler > (http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6710638). > Fixed with > a #ifdef/#ifndef to choose public inheritance when using the Sun > compiler as a workaround (I know, it stinks). But that's probably the best you can do to work around a compiler issue. > 5.- The Uuid.h solaris non-const members. I don't know why Solaris > doesn't define some arguments to these functions as constant, > but they > produce compiler errors. The way I've fixed it was to use the dreaded > #ifndef constructions: > > #ifdef SUNOS > uuid_unparse(const_cast<uint8_t*>(uuid.data()), unparsed); > #else > uuid_unparse(uuid.data(), unparsed); > #endif Right - this is another one that's copied and reimplemented for Windows, so if you need to copy and reimplement for solaris, it's not without precedent. This is a situation, though, where some autoconf feature tests can detect which is needed and set the config appropriately. Then you'd ifdef based oin the feature test rather than a platform/compiler feature. > 6.- Some explicit namespacing. In some parts of the code, I needed to > specify the complete namespace for some calls. I don't know > how it works > on linux, because in most cases, no 'using' clause should > guarantee the > resolution of the namespace. It happened with some algorithms like > for_each or some boost functions. I just added the namespace when the > compiler complained. Ok. > 7.- Replace all the u_intN_t typenames with uintN_t typenames. The > former ones are not available on solaris. Right - this is being done in the Windows port as well. > 8.- The queue issue. Some solaris header defines a struct name as > 'queue'. Usage of that name in constructions like: > > session.queueDeclare(queue=q); > session.messageSubscribe(queue=q, destination=dest, acquireMode=1); > > fires a compiler error, presumably because queue is a > struct name. To > avoid that, I decided to change that usage to: > > session.queueDeclare(arg::queue=q); > session.messageSubscribe(arg::queue=q, arg::destination=dest, > arg::acquireMode=1); > > Not the cleanest one, I know. Any idea to improve it? The only alternative here is to change the name to something other than queue... This problem will come back in other ports. > That's all, I think. Comments about the issues and the way > the've been fixed are welcomed. Very nice. One issue that will keep coming up as more ports are worked on is issues similar to sched_yield and uuid above... Very minor differences in platforms or arguments or standards implementations that really don't warrant reimplementing code. I'd urge some more usage of autoconf feature tests to work these out at build time. I definitely hear Andrew's sentiment that once ifdefs are "ok" they spread like wildfire. The danger in copy/reimplement, however, is also real - the more copies of code there are, the easier it is for them to get out of sync - maintenance becomes harder and harder. There should be a balance between these approaches. Fortunately, the qpid dev team is alert, attentive, and able to review changes that come in so they can watch for excesses in either direction. > Proudly attaching a console output of the 'make check' command: Wonderful - congratulations! -Steve
