hds: Change the way fortran locators work

Locators in Fortran are (oddly) string buffers and not simple
integers.

Previously the contents of "struct LOC" was copied into the buffer
on "export" and copied from the buffer on "import". This worked
okay except that the size of the struct governed the value of
DAT__SZLOC. Any changes meant a full recompile. It also seems
excessive to risk have each locator use tens of bytes of string
buffer when only a few bytes are really needed.

This current implementation stores the pointer in the Fortran
buffer and reads the pointer out of the buffer on import. It's
not very efficient as it currently really does store a formatted
version of the pointer (via sprintf %p) and uses strtol() to
convert that number back to a pointer. It does work though and
DAT__SZLOC is now invariant to the structure size. This will
be important for HDS v5.

Obviously at some point we should just store the 8 bytes into
the buffer and cast the 8 bytes back to a pointer again but
first having to check that the string locators (<NOT A LOCATOR>)
still work.

Note that this patch adds a dependency to ONE. This may just
be temporary.

libraries/hds/Makefile.am

@@ -25,7 +25,7 @@ libhdsf_la_SOURCES = \
 	$(F_C_ROUTINES) \
 	$(BLOCK_DATA_FILES)
 
-libhdsf_la_LIBADD = `ems_link`
+libhdsf_la_LIBADD = `ems_link` `one_link`
 
 # Make all library code position independent. This is handy for creating
 # shareable libraries from the static ones (Java JNI libraries).

libraries/hds/dat1.h

@@ -407,8 +407,6 @@ if (!_ok(*status))\
       int hds1_get_subs( int ndim, HDS_PTYPE *dims, INT_BIG offset,
                          HDS_PTYPE  *subs );
 
-      int dat1_import_floc ( const char flocator[DAT__SZLOC],
-			      int loc_length, HDSLoc *clocator,
-			      int * status);
+      HDSLoc * dat1_import_floc ( const char flocator[DAT__SZLOC], int loc_length, int * status);
 
 #endif

libraries/hds/dat1_import_floc.c

@@ -23,28 +23,26 @@
  *    Import a fortran HDS locator buffer into C
 
  *  Invocation:
- *    dat1_import_floc( const char flocator[DAT__SZLOC], int len, HDSLoc *clocator, int * status);
+ *    clocator = dat1_import_floc( const char flocator[DAT__SZLOC], int len, int * status);
 
  *  Description:
  *    This function should be used to convert a Fortran HDS locator
- *    (implemented as a string buffer) to a C locator struct. It is
- *     for internal usage by HDS only. The public version is datImportFloc.
+ *    (implemented as a string buffer) to a C locator struct.
 
  *  Arguments
  *    flocator = const char * (Given)
  *       Fortran character string buffer. Should be at least DAT__SZLOC
  *       characters long.
  *    len = int (Given)
  *       Size of Fortran character buffer. Sanity check.
- *    clocator = HDSLoc * (Returned)
- *       Fills the supplied HDSLoc struct with the contents of the fortran buffer.
- *       The C struct will not be malloced by this routine.
  *    status = int * (Given and Returned)
  *       Inherited status. Attempts to execute even if status is not DAT__OK
  *       on entry.
 
- *  Returned:
- *    int status = Inherited status value
+ *  Returned Value:
+ *    clocator = HDSLoc *
+ *       C HDS locator corresponding to the Fortran locator.
+ *       Should be freed by using datAnnul().
 
  *  Authors:
  *    Tim Jenness (JAC, Hawaii)
@@ -55,17 +53,25 @@
  *      Initial version
  *    27-JAN-2006 (DSB)
  *      Attempt to execute even if status is set on entry.
+ *    2014-09-07 (TIMJ):
+ *      Complete rewrite to now extract pointer value from buffer. API change
+ *      to return the pointer directly.
 
  *  Notes:
- *    Does not check the contents of the locator for validity but does check for
- *    common Fortran error locators such as DAT__ROOT and DAT__NOLOC.
+ *    - Does not check the contents of the locator for validity but does check for
+ *      common Fortran error locators such as DAT__ROOT and DAT__NOLOC.
+ *    - For internal usage by HDS only. The public version is datImportFloc.
+ *    - Differs from the original HDS API in that it returns the locator.
+ *    - Attempts to execute even if status is bad.
+ *    - This change has been made to allow the struct to be extended without forcing
+ *      a recompile when DAT__SZLOC changes.
 
  *  See Also:
  *    - datImportFloc
  *    - datExportFloc
- *    - dat1_free_hdsloc
 
  *  Copyright:
+ *    Copyright (C) 2014 Cornell University
  *    Copyright (C) 2005 Particle Physics and Astronomy Research Council.
  *    All Rights Reserved.
 
@@ -91,43 +97,62 @@
  *-
  */
 
-int dat1_import_floc ( const char flocator[DAT__SZLOC], int loc_length, HDSLoc * clocator, int * status) {
+HDSLoc *
+dat1_import_floc ( const char flocator[DAT__SZLOC], int loc_length, int * status) {
 
-/* Validate the locator length.                                             */
+  long ptr_as_long = 0;
+  HDSLoc * clocator = NULL;
+
+  /* Validate the locator length. */
   if (loc_length != DAT__SZLOC ) {
     if (*status == DAT__OK ) {
        *status = DAT__LOCIN;
-       emsSeti( "LEN", loc_length );
-       emsSeti( "SZLOC", DAT__SZLOC );
-       emsRep( "DAT1_IMPORT_FLOC", "Locator length is ^LEN not ^SZLOC", status);
+       emsRepf( "DAT1_IMPORT_FLOC", "Locator length is %d not %d", status,
+                loc_length, DAT__SZLOC);
     }
-    return *status;
+    return NULL;
   };
 
   /* Check obvious error conditions */
   if (strncmp( DAT__ROOT, flocator, loc_length) == 0 ){
     if( *status == DAT__OK ) {
        *status = DAT__LOCIN;
-       emsRep( "datImportFloc_ROOT", "Input HDS Locator corresponds to DAT__ROOT but that can only be used from NDF", status );
+       emsRep( "dat1ImportFloc_ROOT",
+               "Input HDS Locator corresponds to DAT__ROOT but that can only be used from NDF",
+               status );
     }
-    return *status;
+    return NULL;
   }
 
   /* Check obvious error conditions */
   if (strncmp( DAT__NOLOC, flocator, loc_length) == 0 ){
     if( *status == DAT__OK ) {
        *status = DAT__LOCIN;
-       emsRep( "datImportFloc_NOLOC", "Input HDS Locator corresponds to DAT__NOLOC but status is good (Possible programming error)", status );
+       emsRep( "datImportFloc_NOLOC",
+               "Input HDS Locator corresponds to DAT__NOLOC but status is good (Possible programming error)",
+               status );
     }
-    return *status;
+    return NULL;
   }
 
-/* If OK, then extract the information from the locator string (necessary   */
-/* to ensure that data alignment is correct, as the string will normally be */
-/* stored externally in a Fortran CHARACTER variable).                      */
-/* We do this copy regardless of status since this is sometimes required
-   and it can't hurt if we have allocated memory                            */
-  memmove( clocator, flocator, sizeof( struct LOC ) );
+  /* Everything seems to be okay so now convert the string buffer to the
+     required pointer. We ignore status as sometimes we need to try
+     to get the value regardless (otherwise DAT_ANNUL from Fortran would
+     never succeed). */
+
+  ptr_as_long = strtol( flocator, NULL, 16 );
+
+  if (ptr_as_long == 0) {
+    /* This should not have happened */
+    if (*status == DAT__OK) {
+      *status = DAT__LOCIN;
+      emsRep("dat1_import_floc_3",
+             "Error importing locator from Fortran", status );
+      return NULL;
+    }
+  }
 
-  return *status;
+  /* Do the cast */
+  clocator = (HDSLoc *)ptr_as_long;
+  return clocator;
 }

libraries/hds/datExportFloc.c

@@ -7,6 +7,7 @@
 #include <string.h>
 
 #include "ems.h"
+#include "star/one.h"
 
 #include "hds1.h"
 #include "rec.h"
@@ -27,6 +28,9 @@
  *  Language:
  *    Starlink ANSI C
 
+ *  Type of Module:
+ *    Library routine
+
  *  Invocation:
  *    datExportFloc( HDSLoc** clocator, int free, int len, char flocator[DAT__SZLOC], int * status );
 
@@ -37,33 +41,41 @@
  *    HDS_EXPORT_CLOCATOR macro defined in hds_fortran.h.
 
  *  Arguments:
- *    HDSLoc ** clocator = Given & Returned
- *       Pointer to Locator to be exported. If the memory is freed, the
- *       locator will be set to NULL on return, else it will be untouched.
- *    int free = Given
- *       If true (1) the C locator is freed once the
+ *    clocator = HDSLoc ** (Given and Returned)
+ *       Pointer to the locator to be exported. See the "free" description below
+ *       as to whether this locator will be annulled or not.
+ *    free = int (Given)
+ *       If true (1) the C locator is nulled out once the
  *       Fortran locator is populated. If false, the locator memory is not
- *       touched.
- *    int len = Given
- *       Size of Fortran character buffer to recieve the locator. Sanity check.
- *    char flocator[DAT__SZLOC] = Returned
+ *       touched. Regardless, the locator itself is not annulled as it is
+ *       now simply referenced from the Fortran.
+ *    len = int (Given)
+ *       Size of Fortran character buffer to receive the locator. Sanity check.
+ *       Should be DAT__SZLOC.
+ *    flocator = char [DAT__SZLOC] (Returned)
  *       Fortran character string buffer. Should be at least DAT__SZLOC
  *       characters long. If clocator is NULL, fills the buffer
  *       with DAT__NOLOC.
- *    int * status = Given & Returned
+ *    status = int* (Given and Returned)
  *       Inherited status. If status is bad the Fortran locator will be
  *       filled with DAT__NOLOC. The memory associated with clocator will
  *       be freed if free is true regardless of status.
 
  *  Authors:
  *    TIMJ: Tim Jenness (JAC, Hawaii)
+ *    TIMJ: Tim Jenness (Cornell)
+ *    {enter_new_authors_here}
 
  *  History:
  *    16-NOV-2005 (TIMJ):
  *      Initial version
  *    18-NOV-2005 (TIMJ):
  *      Make semi public to allow other Fortran wrappers to use this function.
  *      Rename from dat1_export_floc to datExportFloc
+ *    2014-09-07 (TIMJ):
+ *      Rewrite to store the pointer (as string) directly in the Fortran
+ *      string buffer rather than the contents of the struct.
+ *     {enter_further_changes_here}
 
  *  Notes:
  *    - Fortran locator string must be preallocted. The C locator can be freed
@@ -73,11 +85,17 @@
  *    wrapping Fortran layers from C. "Export" means to export a native
  *    C locator to Fortran.
  *    - There is no Fortran eqiuvalent to this routine.
+ *    - This routine differs from the previous HDS implementation in that the address
+ *    of the supplied pointer is stored in the Fortran string buffer and not the contents
+ *    of the struct. This is done to constrain the required size of the locator
+ *    in Fortran to allow the locator structure to change without forcing a full recompile
+ *    of everything.
 
  *  See Also:
  *    datImportFloc
 
  *  Copyright:
+ *    Copyright (C) 2014 Cornell University
  *    Copyright (C) 2005 Particle Physics and Astronomy Research Council.
  *    All Rights Reserved.
 
@@ -105,27 +123,36 @@
 
 void datExportFloc ( HDSLoc **clocator, int free, int loc_length, char flocator[DAT__SZLOC], int * status) {
 
-/* Validate the locator length.                                             */
+  /* Validate the locator length */
   if (*status == DAT__OK && loc_length != DAT__SZLOC ) {
     *status = DAT__LOCIN;
-    emsSeti( "LEN", loc_length );
-    emsSeti( "SZLOC", DAT__SZLOC );
-    emsRep( "datExportFloc", "Locator length is ^LEN not ^SZLOC", status);
-  };
-
-/* If OK, then extract the information from the locator string (necessary   */
-/* to ensure that data alignment is correct, as the string will normally be */
-/* stored externally in a Fortran CHARACTER variable).                      */
+    emsRepf( "datExportFloc", "Locator length is %d not %d", status,
+            loc_length, DAT__SZLOC);
+  }
 
+  /* if everything is okay we store the pointer location in the Fortran
+     locator */
   if ( *status == DAT__OK && *clocator != NULL ) {
-    memmove( flocator, *clocator, sizeof( struct LOC ) );
+
+    /* We export from C by storing the pointer of the C struct in the
+       Fortran character buffer. We can not store a clone in the Fortran
+       locator because clones are documented to not clone mapped status
+       and DAT_MAP / DAT_UNMAP will fail for clones. We just store the
+       supplied locator and null out the C version if we are being requested
+       to free it. Note that if we free=false the caller should not then
+       annul the locator as that would mess up the Fortran side. If the current
+       scheme does not work, we could try assigning the clone to the caller and
+       the original to the fortran locator but this requires some thought. */
+
+    one_snprintf(flocator, loc_length, "%p", status, *clocator );
+
   } else {
     strncpy( flocator, DAT__NOLOC, DAT__SZLOC );
   }
 
-
-  /* Free regardless of status */
-  if (free) dat1_free_hdsloc( clocator );
+  /* Null out the caller if requested. Do not annul as we have stored
+     the original pointer in the Fortran layer */
+  if (free) *clocator = NULL;
 
   return;
 }