ECC-1126: BUFR: ECMWF Local use section key 'ident' should be trimmed (Part 2)

src/bufr_util.c

@@ -204,6 +204,8 @@ static int bufr_decode_rdb_keys(const void* message, long offset_section2, codes
     return GRIB_SUCCESS;
 }
 
+#define IDENT_LEN 9 /* 8 chars plus the final 0 terminator */
+
 /* The ECMWF BUFR local use section */
 static int bufr_decode_extra_rdb_keys(const void* message, long offset_section2, codes_bufr_header* hdr)
 {
@@ -261,8 +263,11 @@ static int bufr_decode_extra_rdb_keys(const void* message, long offset_section2,
         }
     }
     else {
-        size_t i = 0, j = 0;
+        size_t i = 0;
         long lValue  = 0;
+        char* pTemp = NULL;
+        char temp[IDENT_LEN] = {0,};
+
         start               = 72;
         lValue              = (long)grib_decode_unsigned_long(pKeyData, &start, 25);
         hdr->localLatitude  = (lValue - 9000000.0) / 100000.0;
@@ -270,17 +275,14 @@ static int bufr_decode_extra_rdb_keys(const void* message, long offset_section2,
         lValue              = (long)grib_decode_unsigned_long(pKeyData, &start, 26);
         hdr->localLongitude = (lValue - 18000000.0) / 100000.0;
 
-        /* interpret keyMore as a string */
+        /* interpret keyMore as a string. Copy to a temporary */
         for (i = 0; i < 8; ++i) {
-            const char c = *pKeyMore;
-            //printf("Lookin at %c, i=%lu, j=%lu\n", c, i, j);
-            if (c != ' ') {
-                //printf("  not space so copy to %lu\n", j);
-                hdr->ident[j++] = c;
+            temp[i] = *pKeyMore++;
         }
-            pKeyMore++;
-        }
-        hdr->ident[j] = '\0';
+        temp[i] = '\0';
+        pTemp = temp;
+        lrtrim(&pTemp, 1, 1); /* Trim left and right */
+        strncpy(hdr->ident, pTemp, 8);
     }
 
     return GRIB_SUCCESS;

src/grib_accessor_class_trim.c

@@ -18,8 +18,8 @@
    IMPLEMENTS = unpack_string;pack_string
    IMPLEMENTS = init
    MEMBERS=  const char* input
-   MEMBERS=  int trimleft
-   MEMBERS=  int trimright
+   MEMBERS=  int trim_left
+   MEMBERS=  int trim_right
    END_CLASS_DEF
 
  */
@@ -46,8 +46,8 @@ typedef struct grib_accessor_trim
     /* Members defined in ascii */
     /* Members defined in trim */
     const char* input;
-    int trimleft;
-    int trimright;
+    int trim_left;
+    int trim_right;
 } grib_accessor_trim;
 
 extern grib_accessor_class* grib_accessor_class_ascii;
@@ -142,26 +142,10 @@ static void init(grib_accessor* a, const long l, grib_arguments* arg)
     grib_handle* h           = grib_handle_of_accessor(a);
 
     self->input     = grib_arguments_get_name(h, arg, n++);
-    self->trimleft = grib_arguments_get_long(h, arg, n++);
-    self->trimright= grib_arguments_get_long(h, arg, n++);
-    DebugAssert(self->trimleft == 0 || self->trimleft == 1);
-    DebugAssert(self->trimright == 0 || self->trimright == 1);
-}
-
-static void trim(char** x)
-{
-    char* p = 0;
-    while (**x == ' ' && **x != '\0')
-        (*x)++;
-    if (**x == '\0')
-        return;
-    p = (*x) + strlen(*x) - 1;
-    while (*p == ' ') {
-        *p = '\0';
-        p--;
-    }
-    if (*p == ' ')
-        *p = '\0';
+    self->trim_left = grib_arguments_get_long(h, arg, n++);
+    self->trim_right= grib_arguments_get_long(h, arg, n++);
+    DebugAssert(self->trim_left == 0 || self->trim_left == 1);
+    DebugAssert(self->trim_right == 0 || self->trim_right == 1);
 }
 
 static int unpack_string(grib_accessor* a, char* val, size_t* len)
@@ -176,13 +160,12 @@ static int unpack_string(grib_accessor* a, char* val, size_t* len)
 
     err = grib_get_string(h, self->input, input, &size);
     if (err) return err;
-    DebugAssert(size < 9);
 
-    trim(&pInput);
+    lrtrim(&pInput, self->trim_left, self->trim_right);
     sprintf(val, "%s", pInput); 
     size = strlen(val);
     *len = size + 1;
-    return err;
+    return GRIB_SUCCESS;
 }
 
 static int pack_string(grib_accessor* a, const char* val, size_t* len)
@@ -206,7 +189,7 @@ static int pack_string(grib_accessor* a, const char* val, size_t* len)
     
     sprintf(buf, "%s", val);
     pBuf = buf;
-    trim(&pBuf);
+    lrtrim(&pBuf, self->trim_left, self->trim_right);
 
     return grib_pack_string(inputAccesstor, pBuf, len);
 }

src/grib_api_prototypes.h

@@ -1494,6 +1494,7 @@ int codes_bufr_header_get_string(codes_bufr_header* bh, const char* key, char* v
 /* string_util.c */
 int strcmp_nocase(const char* s1, const char* s2);
 void rtrim(char* s);
+void lrtrim(char** x, int do_left, int do_right);
 const char* extract_filename(const char* filepath);
 char** string_split(char* inputString, const char* delimiter);
 int string_to_long(const char* input, long* output);

src/grib_dumper_class_json.c

@@ -460,7 +460,6 @@ static void dump_string(grib_dumper* d, grib_accessor* a, const char* comment)
         /* ECC-356: Solution for the special local section key 'keyMore' and its alias 'ident' */
         int skip = 1;
         if ((a->flags & GRIB_ACCESSOR_FLAG_HIDDEN) != 0) {
-            //grib_handle* h = grib_handle_of_accessor(a);
             if (strcmp(a->name, "identTrimmed") == 0) {
                 skip     = 0;
                 acc_name = "ident";

src/string_util.c

@@ -38,6 +38,26 @@ void rtrim(char* s)
     s[len] = '\0';
 }
 
+void lrtrim(char** x, int do_left, int do_right)
+{
+    DebugAssert(do_left || do_right);
+    if (do_left) {
+        while (isspace(**x) && **x != '\0')
+            (*x)++;
+    }
+    if (**x == '\0')
+        return;
+    if (do_right) {
+        char* p = (*x) + strlen(*x) - 1;
+        while (isspace(*p)) {
+            *p = '\0';
+            p--;
+        }
+        if (isspace(*p))
+            *p = '\0';
+    }
+}
+
 /* Return the component after final slash */
 /*  "/tmp/x"  -> "x"  */
 /*  "/tmp/"   -> ""   */

tests/unit_tests.c

@@ -1485,9 +1485,41 @@ static void test_concept_condition_strings()
     grib_handle_delete(h);
 }
 
+void test_trimming()
+{
+    char a[] = " Standing  ";
+    char b[] = "  Weeping ";
+    char c[] = "  Silhouette ";
+    char d[] = " The Forest Of October  ";
+    char e[] = "\t\n Apostle In Triumph \r ";
+    char* pA = a;
+    char* pB = b;
+    char* pC = c;
+    char* pD = d;
+    char* pE = e;
+
+    lrtrim(&pA, 0, 1); /*right only*/
+    assert( strcmp(pA, " Standing")==0 );
+
+    lrtrim(&pB, 1, 0); /*left only*/
+    assert( strcmp(pB, "Weeping ")==0 );
+
+    lrtrim(&pC, 1, 1); /*both ends*/
+    assert( strcmp(pC, "Silhouette")==0 );
+
+    lrtrim(&pD, 1, 1); /*make sure other spaces are not removed*/
+    assert( strcmp(pD, "The Forest Of October")==0 );
+
+    lrtrim(&pE, 1, 1); /* Other chars */
+    assert( strcmp(pE, "Apostle In Triumph")==0 );
+}
+
 int main(int argc, char** argv)
 {
     /*printf("Doing unit tests. ecCodes version = %ld\n", grib_get_api_version());*/
+    
+    test_trimming();
+
     test_get_git_sha1();
 
     test_concept_condition_strings();