[gdal-dev] S57 field definition incorrect

[ogr user]

In case anyone is interested, here is a patch that adds appropriate
handling of IntegerList field types to S57 parser. For backwards
compatibility it is only activated if specific option is passed to the
reader (actually to class registrar). This is for 1.5.3.

Frank Warmerdam wrote:
>
> Dear OGR User,
>
> It appears the S57 code deliberately treats list attributes as simple
> strings.  While this may have been a poor choice, I hesitate to change it
> at this point for fear of breaking fragile software built on this driver
> (notably the FME S-57 reader).
>
> My suggestion is that you fetch it as a string, and parse the integers
> out yourself.
>
> Best regards,


-------------- next part --------------
Index: gdal-1.5.3/ogr/ogrutils.cpp
===================================================================
--- gdal-1.5.3/ogr/ogrutils.cpp	
+++ gdal-1.5.3/ogr/ogrutils.cpp	
@@ -817,3 +817,41 @@
 
     return TRUE;
 }
+
+
+#define P_DELIM   " ,"
+int OGRParseIntegerList(const char *pszInput, OGRField *psField)
+{
+   const char *p;
+   int nCount, nLen, sLen;
+   int *pVals;
+
+   p = pszInput;
+   nCount = 0;
+   while ((nLen = strcspn(p, P_DELIM)) > 0) {
+      nCount++;
+      p += nLen;
+      sLen = strspn(p, P_DELIM);
+      p += sLen;
+   }
+
+   if (nCount == 0)
+      return 0;
+
+   pVals = (int *)CPLMalloc(sizeof(int) * nCount);
+
+   p = pszInput;
+   nCount = 0;
+   while ((nLen = strcspn(p, P_DELIM)) > 0) {
+      pVals[nCount] = atoi(p);
+      nCount++;
+      p += nLen;
+      sLen = strspn(p, P_DELIM);
+      p += sLen;
+   }
+
+   psField->IntegerList.paList = pVals;
+   psField->IntegerList.nCount = nCount;
+
+   return 1;
+}
Index: gdal-1.5.3/ogr/ogrsf_frmts/s57/s57reader.cpp
===================================================================
--- gdal-1.5.3/ogr/ogrsf_frmts/s57/s57reader.cpp	
+++ gdal-1.5.3/ogr/ogrsf_frmts/s57/s57reader.cpp	
@@ -246,6 +246,14 @@
 /*                             SetOptions()                             */
 /************************************************************************/
 
+void S57Reader::SetOptions( int nFlagsOn, int nFlagsOff )
+{
+   nOptionFlags |= nFlagsOn;
+   if (nFlagsOff)
+      nOptionFlags &= ~nFlagsOff;
+}
+
+
 void S57Reader::SetOptions( char ** papszOptionsIn )
 
 {
@@ -254,6 +262,12 @@
     CSLDestroy( papszOptions );
     papszOptions = CSLDuplicate( papszOptionsIn );
 
+    pszOptionValue = CSLFetchNameValue( papszOptions, S57O_PARSE_INTEGERLIST );
+    if( pszOptionValue != NULL && !EQUAL(pszOptionValue,"OFF") )
+        nOptionFlags |= S57M_PARSE_INTEGERLIST;
+    else
+        nOptionFlags &= ~S57M_PARSE_INTEGERLIST;
+
     pszOptionValue = CSLFetchNameValue( papszOptions, S57O_SPLIT_MULTIPOINT );
     if( pszOptionValue != NULL && !EQUAL(pszOptionValue,"OFF") )
         nOptionFlags |= S57M_SPLIT_MULTIPOINT;
Index: gdal-1.5.3/ogr/ogrsf_frmts/s57/s57featuredefns.cpp
===================================================================
--- gdal-1.5.3/ogr/ogrsf_frmts/s57/s57featuredefns.cpp	
+++ gdal-1.5.3/ogr/ogrsf_frmts/s57/s57featuredefns.cpp	
@@ -416,7 +416,10 @@
             break;
 
           case SAT_LIST:
-            oField.SetType( OFTString );
+            if (nOptionFlags & S57M_PARSE_INTEGERLIST)
+               oField.SetType( OFTIntegerList );
+            else
+               oField.SetType( OFTString );
             break;
         }
 
Index: gdal-1.5.3/ogr/ogrsf_frmts/s57/s57.h
===================================================================
--- gdal-1.5.3/ogr/ogrsf_frmts/s57/s57.h	
+++ gdal-1.5.3/ogr/ogrsf_frmts/s57/s57.h	
@@ -52,6 +52,7 @@
 #define S57O_RETURN_PRIMITIVES "RETURN_PRIMITIVES"
 #define S57O_RETURN_LINKAGES "RETURN_LINKAGES"
 #define S57O_RETURN_DSID     "RETURN_DSID"
+#define S57O_PARSE_INTEGERLIST   "PARSE_INTEGERLIST"
 
 #define S57M_UPDATES                    0x01
 #define S57M_LNAM_REFS                  0x02
@@ -61,6 +62,7 @@
 #define S57M_RETURN_PRIMITIVES          0x20
 #define S57M_RETURN_LINKAGES            0x40
 #define S57M_RETURN_DSID                0x80
+#define S57M_PARSE_INTEGERLIST        0x0100
 
 /* -------------------------------------------------------------------- */
 /*      RCNM values.                                                    */
@@ -291,6 +293,7 @@
 
     void                SetClassBased( S57ClassRegistrar * );
     void                SetOptions( char ** );
+    void                SetOptions( int nFlagsOn, int nFlagsOff );
     int                 GetOptionFlags() { return nOptionFlags; }
 
     int                 Open( int bTestOpen );
Index: gdal-1.5.3/ogr/ogrfeature.cpp
===================================================================
--- gdal-1.5.3/ogr/ogrfeature.cpp	
+++ gdal-1.5.3/ogr/ogrfeature.cpp	
@@ -1860,6 +1860,16 @@
         if( OGRParseDate( pszValue, &sWrkField, 0 ) )
             memcpy( pauFields+iField, &sWrkField, sizeof(sWrkField));
     }
+    else if (poFDefn->GetType() == OFTIntegerList) {
+        OGRField sWrkField;
+
+        if (OGRParseIntegerList(pszValue, &sWrkField)) {
+            if (IsFieldSet(iField))
+               CPLFree(pauFields[iField].IntegerList.paList);
+
+            memcpy(pauFields+iField, &sWrkField, sizeof(sWrkField));
+        }
+    }
     else
         /* do nothing for other field types */;
 }
Index: gdal-1.5.3/ogr/ogr_core.h
===================================================================
--- gdal-1.5.3/ogr/ogr_core.h	
+++ gdal-1.5.3/ogr/ogr_core.h	
@@ -292,6 +292,8 @@
 int CPL_DLL OGRParseDate( const char *pszInput, OGRField *psOutput, 
                           int nOptions );
 
+int CPL_DLL OGRParseIntegerList(const char *pszInput, OGRField *psField);
+
 /* -------------------------------------------------------------------- */
 /*      Constants from ogrsf_frmts.h for capabilities.                  */
 /* -------------------------------------------------------------------- */