Update g2_gribend / g2c_check_msg for rare condition

[EricEngle-NOAA]

I came across a rare condition when addressing a grib2io issue. In short, the user was trying to pack data using simple packing, no decimal or binary scaling, and nbits=0. This results in the packing the data to the nearest integer. Just so happens that the last 4 values of the grid rounded to a value of 55, which resolves to "7" in the ASCII table. In this scenario, g2c thinks the GRIB2 message is complete and gives a non-zero return status.

Therefore, I think g2_gribend and/or g2c_check_msg need to check for this rare condition.

Thinking about this a bit, perhaps g2c_check_msg should check the length of section 7 and if the last 4 bytes are "7777" (assuming section length is correct), then to not trigger the error.

[edwardhartnett]

Maybe we should just cut this code:

/* Check to see if GRIB message is already complete. */
if (cgrib[*lencurr - 4] == seven && cgrib[*lencurr - 3] == seven &&
    cgrib[*lencurr - 2] == seven && cgrib[*lencurr - 1] == seven)
{
    if (verbose)
        printf("GRIB message already complete.  Cannot add new section.\n");
    return G2C_EMSGCOMPLETE;
}

[EricEngle-NOAA]

I think the above is needed. I am looking at g2_gribend. With some additional logic, we can check for this. I can work on this.

[edwardhartnett]

If you have a solution, that would be great. I didn't see an easy way to do that...

[EricEngle-NOAA]

Lets table this for v1.10

[edwardhartnett]

The next version (v2.0.0) will be released soon. Should this be bumped to the next release?

[EricEngle-NOAA]

I have talked about this rare condition on a few occasions with different groups of people. Someone suggested that if the last 4 bytes of the packed data are [55, 55, 55, 55] (i.e. "7777"), then just make the last byte either 54 or 56. In theory, this byte would only impact a corner of the grid.

Thoughts?

[edwardhartnett]

Don't we know the size of the section anyway? In other words do we need to scan for 7777?

[EricEngle-NOAA]

Continuing to look at this... g2_gribend is performing checking actions after the call to g2c_check_msg. So perhaps, we should move some of the checking logic from g2_gribend into g2c_check_msg -- mainly this section:

    /*  Loop through all current sections of the GRIB message to find
     *  the last section number. */
    len = 16;    /* Length of Section 0. */
    for (;;)
    {
        /*    Get number and length of next section. */
        iofst = len * 8;
        gbit(cgrib, &ilen, iofst, 32);
        iofst = iofst + 32;
        gbit(cgrib, &isecnum, iofst, 8);
        len = len + ilen;

        /*    Exit loop if last section reached. */
        if (len == lencurr)
            break;

        /*    If byte count for each section doesn't match current
         *    total length, then there is a problem. */
        if (len > lencurr)
        {
            printf("g2_gribend: Section byte counts don''t add to total.\n");
            printf("g2_gribend: Sum of section byte counts  =  %d\n", (int)len);
            printf("g2_gribend: Total byte count in Section 0 = %d\n", (int)lencurr);
            return G2_BAD_SEC_COUNTS;
        }
    }

If you look at g2c_check_msg, it is only looking at the first and last 4 bytes, but does not check section order and lengths. IMO it should.

And perhaps it should conditionally check for the end section? The modified workflow in g2_gribend would then be:

...input cgrib has all sections and ready to add end...
call g2c_check_msg (conditional arg to not check for end)
if OK
   add end section
   check again (conditional arg to now check for end)