Starlink / starlink

Starlink Software Collection
162 stars 53 forks source link

hlp: crehlp no longer works on OS X Mavericks #29

Closed timj closed 10 years ago

timj commented 10 years ago

The crehlp command no longer works on OS X Mavericks. The reason is that strncpy is being used to copy characters within the same buffer and the behavior of this is undefined. This is the code from creh.c:

   382              iobuf [ 0 ] = (char) c1;
   383              iobuf [ 1 ] = (char) ' ';
   384              l = ito - ifrom + 1;
-> 385              strncpy ( iobuf + 2, iobuf + ifrom, l );
   386              for ( i = 2 + l; i < LIN - 1; iobuf [ i++ ] = (char) ' ' );
   387              iobuf [ i ] = (char) '\0';

In line 385 we copy characters from iobuf into iobuf. For the example in convert/tasks.hlp ifrom has a value of 2 and so this line ends up copying the contents from the same position. It seems that Mavericks now has overlap detection code in place that triggers an abort at runtime. It seems that if we are really intending to shift characters from the end of iobuf to position 2 of iobuf then we should probably change the code to use memmove and terminate the resultant buffer ourself at position l+2:

diff --git a/libraries/hlp/creh.c b/libraries/hlp/creh.c
index c9912f1..e139c2a 100644
--- a/libraries/hlp/creh.c
+++ b/libraries/hlp/creh.c
@@ -382,7 +382,8 @@ int hlpCreh ( int ( * nametr ) ( int, char*, int, char* ),
                iobuf [ 0 ] = (char) c1;
                iobuf [ 1 ] = (char) ' ';
                l = ito - ifrom + 1;
-               strncpy ( iobuf + 2, iobuf + ifrom, l );
+               memmove( &(iobuf[2]), &(iobuf[ifrom]), l );
+               iobuf[l+2] = '\0';
                for ( i = 2 + l; i < LIN - 1; iobuf [ i++ ] = (char) ' ' );
                iobuf [ i ] = (char) '\0';

The above patch seems to be enough to get help library created again.

The build system doesn't seem to care that crehlp is failing, and I think that is because of a related bug in hlib where the return value from crehlp is not trapped:

foreach file ($*)
   crehlp $file $file:r.shl
end
timj commented 10 years ago

c220348488bb9916a083dbe3b85f1a7a593be008 seems to have fixed the abort on OS X but has broken HLP. For example:

$ kaphelp 

 Hel

    Welcome to the KAPPA online help system.  Here you can display
    details about specific KAPPA commands. For more general information
    such as using this help system, consult SUN/95. This can be most easily
    done using the command "showme sun95".

   Additional information available:

   AD         APERAD     ARDGE      ARDMAS     ARDPLO     AXCON      AXLABE     AXUNIT
   CAL        CALPO      CARPE      CDI        CENTROI    CHAI       CHANMA     CHPI

so the final character is being removed from each section. Selecting a section continues to indicate the truncation:

Additional information available:

Parameter  Example    Note       Related_Application   Copyrigh   Licenc     Author     Histor

I wonder if the new hlpStrncp is not working properly.

timj commented 10 years ago

We have decided to leave the memmove() patch in place and not attempt further upgrades of HLP.