Merge pull request #184 from ecmwf/bugfix/ECC-1741-LargeGrib1

Bugfix/ECC-1741 large grib1

examples/python/CMakeLists.txt

@@ -3,7 +3,7 @@
 # Configure the file which all CMake tests will include
 configure_file( include.ctest.sh.in  include.ctest.sh  @ONLY )
 
-execute_process( COMMAND ${CMAKE_COMMAND} -E copy_if_different ${CMAKE_CURRENT_SOURCE_DIR}/include.sh ${CMAKE_CURRENT_BINARY_DIR} )
+# execute_process( COMMAND ${CMAKE_COMMAND} -E copy_if_different ${CMAKE_CURRENT_SOURCE_DIR}/include.sh ${CMAKE_CURRENT_BINARY_DIR} )
 
 # Build the executables used by test scripts
 ################################################

src/eccodes_prototypes.h

@@ -801,7 +801,7 @@ void grib_grow_buffer(const grib_context* c, grib_buffer* b, size_t new_size);
 void grib_buffer_set_ulength_bits(const grib_context* c, grib_buffer* b, size_t length_bits);
 void grib_buffer_set_ulength(const grib_context* c, grib_buffer* b, size_t length);
 void grib_recompute_sections_lengths(grib_section* s);
-void grib_buffer_replace(grib_accessor* a, const unsigned char* data, size_t newsize, int update_lengths, int update_paddings);
+int grib_buffer_replace(grib_accessor* a, const unsigned char* data, size_t newsize, int update_lengths, int update_paddings);
 void grib_update_sections_lengths(grib_handle* h);
 
 /* grib_dumper.cc*/

src/grib_accessor_class.cc

@@ -322,7 +322,8 @@ int grib_section_adjust_sizes(grib_section* s, int update, int depth)
                 if (update) {
                     plen = length;
                     lret = grib_pack_long(s->aclength, &plen, &len);
-                    Assert(lret == GRIB_SUCCESS);
+                    if (lret != GRIB_SUCCESS)
+                        return lret;
                     s->padding = 0;
                 }
                 else {

src/grib_accessor_class_data_g1simple_packing.cc

@@ -313,7 +313,8 @@ static int pack_double(grib_accessor* a, const double* cval, size_t* len)
     grib_context_log(a->context, GRIB_LOG_DEBUG,
                      "grib_accessor_data_g1simple_packing : pack_double : packing %s, %d values", a->name, n_vals);
 
-    grib_buffer_replace(a, buf, buflen, 1, 1);
+    ret = grib_buffer_replace(a, buf, buflen, 1, 1);
+    if (ret != GRIB_SUCCESS) return ret;
 
     grib_context_buffer_free(a->context, buf);
 

src/grib_accessor_class_g1_message_length.cc

@@ -210,6 +210,7 @@ static int pack_long(grib_accessor* a, const long* val, size_t* len)
                              " (actual length=%ld)",
                               cclass_name, __func__, *val, total_length);
             grib_context_log(a->context, GRIB_LOG_ERROR, "Hint: Try encoding as GRIB2\n");
+            return GRIB_ENCODING_ERROR;
         }
         Assert(total_length == *val);
     }

src/grib_accessor_class_g1bitmap.cc

@@ -149,7 +149,8 @@ static int pack_double(grib_accessor* a, const double* val, size_t* len)
     if ((err = grib_set_long_internal(grib_handle_of_accessor(a), self->unusedBits, tlen * 8 - *len)) != GRIB_SUCCESS)
         return err;
 
-    grib_buffer_replace(a, buf, tlen, 1, 1);
+    err = grib_buffer_replace(a, buf, tlen, 1, 1);
+    if (err) return err;
 
     grib_context_free(a->context, buf);
 

src/grib_buffer.cc

@@ -195,8 +195,8 @@ static void update_offsets_after(grib_accessor* a, long len)
 //         update_sections_lengths(s->owner->parent);
 // }
 
-void grib_buffer_replace(grib_accessor* a, const unsigned char* data,
+int grib_buffer_replace(grib_accessor* a, const unsigned char* data,
-                         size_t newsize, int update_lengths, int update_paddings)
+                        size_t newsize, int update_lengths, int update_paddings)
 {
     size_t offset = a->offset;
     long oldsize  = grib_get_next_position_offset(a) - offset;
@@ -232,11 +232,13 @@ void grib_buffer_replace(grib_accessor* a, const unsigned char* data,
         update_offsets_after(a, increase);
         if (update_lengths) {
             grib_update_size(a, newsize);
-            grib_section_adjust_sizes(grib_handle_of_accessor(a)->root, 1, 0);
+            int err = grib_section_adjust_sizes(grib_handle_of_accessor(a)->root, 1, 0);
+            if (err) return err;
             if (update_paddings)
                 grib_update_paddings(grib_handle_of_accessor(a)->root);
         }
     }
+    return GRIB_SUCCESS;
 }
 
 void grib_update_sections_lengths(grib_handle* h)

src/grib_value.cc

@@ -755,7 +755,7 @@ int grib_set_double_array_internal(grib_handle* h, const char* name, const doubl
     }
 
     if (ret != GRIB_SUCCESS)
-        grib_context_log(h->context, GRIB_LOG_ERROR, "Unable to set double array %s (%s)",
+        grib_context_log(h->context, GRIB_LOG_ERROR, "Unable to set double array '%s' (%s)",
                          name, grib_get_error_message(ret));
     /*if (h->context->debug) fprintf(stderr,"ECCODES DEBUG grib_set_double_array_internal key=%s --DONE\n",name);*/
     return ret;
@@ -928,7 +928,7 @@ int grib_set_long_array_internal(grib_handle* h, const char* name, const long* v
 {
     int ret = _grib_set_long_array(h, name, val, length, 0);
     if (ret != GRIB_SUCCESS)
-        grib_context_log(h->context, GRIB_LOG_ERROR, "Unable to set long array %s (%s)",
+        grib_context_log(h->context, GRIB_LOG_ERROR, "Unable to set long array '%s' (%s)",
                          name, grib_get_error_message(ret));
     return ret;
 }

tests/CMakeLists.txt

@@ -19,6 +19,7 @@ list(APPEND test_c_bins
     grib_multi_from_message
     grib_clone_headers_only
     grib_read_index
+    grib_set_large_message_fail
     unit_tests
     bufr_keys_iter
     grib_keys_iter
@@ -160,6 +161,7 @@ if( HAVE_BUILD_TOOLS )
     # and/or take much longer
     list(APPEND tests_extra
         grib_data_quality_checks
+        grib_set_large_message_fail
         grib_g1monthlydate
         grib_g1fcperiod
         grib_bpv_limit

tests/grib_png.sh

@@ -53,7 +53,7 @@ infile=${data_dir}/sample.grib2
 set +e
 ${tools_dir}/grib_set -r -s packingType=grid_png $infile $temp > $tempErr 2>&1
 set -e
-grep -q "Unable to set double array codedValues" $tempErr
+grep -q "Unable to set double array 'codedValues'" $tempErr
 
 # Nearest neighbour
 # ----------------------

tests/grib_set_large_message_fail.cc

@@ -0,0 +1,70 @@
+/*
+ * (C) Copyright 2005- ECMWF.
+ *
+ * This software is licensed under the terms of the Apache Licence Version 2.0
+ * which can be obtained at http://www.apache.org/licenses/LICENSE-2.0.
+ *
+ * In applying this licence, ECMWF does not waive the privileges and immunities granted to it by
+ * virtue of its status as an intergovernmental organisation nor does it submit to any jurisdiction.
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#undef NDEBUG
+#include <assert.h>
+
+#include "eccodes.h"
+
+int main(int argc, char** argv)
+{
+    int err = 0, i = 0, NUM_MISSING = 10;
+    codes_handle* h   = NULL;
+    size_t values_len = 0;
+    long Ni = 0, Nj = 0;
+    double* values = NULL;
+    const double missing = 1.0e36;
+    bool use_bitmap = false;
+
+    if (argc == 2 && strcmp(argv[1], "-b")==0) {
+         use_bitmap = true;
+    }
+
+    h = codes_grib_handle_new_from_samples(NULL, "GRIB1");
+    assert(h);
+
+    CODES_CHECK(codes_set_double(h, "missingValue", missing), 0);
+
+    Ni = Nj = 20000;
+    values_len = Ni * Nj;
+
+    values = (double*)calloc(values_len, sizeof(double));
+
+    CODES_CHECK(codes_set_long(h, "Ni", Ni), 0);
+    CODES_CHECK(codes_set_long(h, "Nj", Nj), 0);
+
+    if (use_bitmap) {
+        printf("Adding a bitmap...\n");
+        CODES_CHECK(codes_set_long(h, "bitmapPresent", 1), 0);
+        for (i = 0; i < NUM_MISSING; i++) {
+            values[i] = missing;
+        }
+    } else {
+        printf("Not adding a bitmap...\n");
+        values[0] = 42;
+        values[1] = 52;
+    }
+
+    printf("Setting the values array...\n");
+    err = codes_set_double_array(h, "values", values, values_len);
+    if (err) {
+        printf("codes_set_double_array failed as expected: err=%s\n", codes_get_error_message(err));
+    } else {
+        fprintf(stderr, "Error: codes_set_double_array should have failed!\n");
+        return 1;
+    }
+
+    codes_handle_delete(h);
+    free(values);
+
+    return 0;
+}

tests/grib_set_large_message_fail.sh

@@ -0,0 +1,23 @@
+#!/bin/sh
+# (C) Copyright 2005- ECMWF.
+#
+# This software is licensed under the terms of the Apache Licence Version 2.0
+# which can be obtained at http://www.apache.org/licenses/LICENSE-2.0.
+#
+# In applying this licence, ECMWF does not waive the privileges and immunities granted to it by
+# virtue of its status as an intergovernmental organisation nor does it submit to any jurisdiction.
+#
+
+. ./include.ctest.sh
+
+label='grib_set_large_message_fail_test'
+temp=temp.$label.txt
+
+$EXEC ${test_dir}/grib_set_large_message_fail > $temp 2>&1
+grep -q "Failed to set GRIB1 message length" $temp
+
+$EXEC ${test_dir}/grib_set_large_message_fail -b > $temp 2>&1
+grep -q "Unable to set double array.*bitmap" $temp
+
+# Clean up
+rm -f $temp