Learn how easy it is to sync an existing GitHub or Google Code repo to a SourceForge project! See Demo

Close

#387 Fix compilation with SunStudio12

closed
G Biggs
Player (393)
5
2009-02-17
2009-01-28
Paul Osmialowski
No

This was made for Player-2.2 SVN snapshot taken 2009.01.28. Among all things it fixes, it also tries to solve problem with nanosleep() detection on Solaris system (and any other system on which this function is provided within librt library). Currently it is done by:
@@ -47,7 +47,6 @@

CHECK_FUNCTION_EXISTS (gettimeofday HAVE_GETTIMEOFDAY)
CHECK_FUNCTION_EXISTS (usleep HAVE_USLEEP)
-CHECK_FUNCTION_EXISTS (nanosleep HAVE_NANOSLEEP)
CHECK_FUNCTION_EXISTS (cfmakeraw HAVE_CFMAKERAW)
CHECK_FUNCTION_EXISTS (dirname HAVE_DIRNAME)
CHECK_INCLUDE_FILES (linux/joystick.h HAVE_LINUX_JOYSTICK_H)
@@ -107,6 +106,7 @@
CHECK_LIBRARY_EXISTS (rt clock_gettime "${PLAYER_EXTRA_LIB_DIRS}" HAVE_LIBRT)
SET (CMAKE_REQUIRED_LIBRARIES rt)
CHECK_FUNCTION_EXISTS (clock_gettime HAVE_CLOCK_GETTIME_FUNC)
+CHECK_FUNCTION_EXISTS (nanosleep HAVE_NANOSLEEP)
SET (CMAKE_REQUIRED_LIBRARIES)
IF (HAVE_LIBRT AND HAVE_CLOCK_GETTIME_FUNC)
SET (HAVE_CLOCK_GETTIME TRUE)
- check for nanosleep() is moved from beginning of SearchForStuff.cmake file in place where need for librt is assured.
How come I didn't find this error before? It's simple: nanosleep() replacement is done only for WIN32, so normal nanosleep was used, later discovery of librt is done properly, so everything linked fine. I've found the problem existance only because suncc is more precise than gcc and puts a warning while trying to build unwanted replace/nanosleep.c
Someone more responsible for cmake infrastructure should justify if my solution is proper (i.e. it must be assured that it does not break compatibility with system that does not offer librt!)

Also I did nothing about playerlogsplitter utility. It can't be linked without -lm as it uses function from math.h. I'm sure it is safe to add -lm for logsplitter permanently as all systems I ever ran had mathematical library. Again, I'm leaving the decision for someone more responsible for cmake infrastructure.

One more thing that cmake should do is trying to find if <ieeefp.h> is available, and if so, files that call finite() function should include that (for now it is only server/drivers/localization/amcl/pf/pf_vector.c).

C++ client library errors aren't fixed since I don't know how to do that.

Changes in Create and Roomba drivers are rather heavy, sunCC does not allow to declare non-constant size of an array, so I replaced them with malloc()/free() pairs. Also in roomba driver I've added missing checks for memory allocation errors.

Discussion

    • assigned_to: nobody --> gbiggs
     
    • labels: --> Player
     
  • Later on I was able to compile bt848x driver using Sunstudio12 (see http://bt848x.sourceforge.net\) it exports videodev.h file which is also symlinked as linux/videodev.h file (so cmake can find it!). Howerver, to compile camerav4l driver two changes were needed:
    1. linux/fs.h and linux/kernel.h included by v4lcapture.h are not used and not available for bt848x; bot gcc and sunstudio12 compilers provide 'sun' definition (small cases) on Solaris, so it can be detected by #ifdef/#ifndef
    2. suncc does not like void pointers arirhmetics, cast to unsigned char * was done wrong way in two places inside of v4lcapture.c file
    Following two patches make required corrections, yet another driver is able to build with Sunstudio!

    File Added: v4lcapture.h.diff

     
  •  
    Attachments
  • File Added: v4lcapture.c.diff

     
  • server/drivers/camera/v4l/v4lcapture.c

     
    Attachments
  • After applying little changes I was able to build opencv (together with v4l support thanks to bt848x) using SunStudio12/Solaris10. That uncovered more errors in Player drivers, namely, inside upcbarcode.cc file. Here's the patch that solves all problems.
    File Added: upcbarcode.cc.diff

     
  • player-20090128/server/drivers/blobfinder/upcbarcode/upcbarcode.cc

     
    Attachments
  • G Biggs
    G Biggs
    2009-02-10

    What error are you getting for the "int symbols[][2]" stuff? This is valid C++, as far as I know.

     
  • "int symbols[][2]" stuff is indeed valid, however, it can't be used due to other changes I did into upcbarcode.cc file. All started with this:
    - int symbols[height][2];
    + int ** symbols;
    As you may know that, sunCC does not allow to declare an array which size is unknown at compile time. So I replaced this by dynamically allocated array of pointers to pointers. As a consequence, I had to change type of parameters of ExtractSymbol() and ExtractCode() methods which was "int symbols[][2]".

     
  • G Biggs
    G Biggs
    2009-02-10

    Use the new operator to allocate memory. You can allocate multidimensional arrays with new which will match the function declaration.

     
  • G Biggs
    G Biggs
    2009-02-10

    I have applied most of player-20090128.sunstudio12.patch. There were a couple of bits that looked to be improvements to a driver (skyetekM1) rather than changes to make it compile on Solaris. If this is the case, submit a separate patch. Otherwise, submit an updated patch with the bits I've missed here.

     
  • "new" used instead of "malloc"

     
    Attachments
  • Ok, here it is, upcbarcode.cc with "new" instead of "malloc"
    @skyetekM1: some of the changes were required ("unsigned char TID[len]"), some of them make this driver safer ("if (len > 0)"), however, the change from usleep to nanosleep may seem to go too far, although, one day we will have to drop all signal messing sleeps and usleeps and replace them with safer nanosleeps.

    File Added: upcbarcode.cc.diff

     
  • G Biggs
    G Biggs
    2009-02-17

    I have applied the other two patches. Can you submit the nanosleep changes for the skyetekM1 as a separate patch?

     
  • G Biggs
    G Biggs
    2009-02-17

    The changes have been made in SVN trunk.
    Thank you for your contribution.

     
  • G Biggs
    G Biggs
    2009-02-17

    • status: open --> closed