Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

_FillValue attribute erases previous data #390

Closed
Dave-Allured opened this issue Mar 29, 2017 · 8 comments
Closed

_FillValue attribute erases previous data #390

Dave-Allured opened this issue Mar 29, 2017 · 8 comments
Assignees
Milestone

Comments

@Dave-Allured
Copy link
Contributor

Platform: Mac OS X, 64 bit
Build system: Via Macports.org, uses autotools I think.
netcdf version: 4.4.1

Summary of Issue:

Writing a new _FillValue attribute to a netcdf-4 file variable with previous data should give an error. Instead, the _FillValue is written, and previous data is erased.

Steps to reproduce:

  1. Start with an existing netcdf-4 file, and a variable with previous data, but NO DEFINED _FILLVALUE.
  2. Open file in read/write mode.
  3. Enter redefine mode (required with netcdf-4 classic format, optional with plain netcdf-4).
  4. Call nc_put_att, define a new _FillValue. Error expected here.
  5. Call nc_close.
  6. Compare netcdf files, before and after.

Here is a sample program to write the new _FillValue attribute, thanks to @wkliao whose program from related issue #384 I modified:

#include <stdio.h>
#include <netcdf.h>

#define ERR {if(err!=NC_NOERR)printf("Error at line %d: %s\n",__LINE__,nc_strerror(err));}
int
main (int argc, char **argv)
{
    int ncid, varid, fmode, err;
    int fillv;

    fmode = NC_WRITE;
    err = nc_open ("testfile.nc", fmode, &ncid); ERR
    err = nc_inq_varid (ncid, "var", &varid); ERR

    err = nc_redef (ncid); ERR

    /* try put attribute _FillValue and expect NC_ELATEFILL */
    fillv = 99;
    err = nc_put_att_int (ncid, varid, _FillValue, NC_INT, 1, &fillv);
    if (err != NC_ELATEFILL)
        printf("line %d expecting NC_ELATEFILL but got %d\n",__LINE__,err);
    err = nc_close (ncid); ERR
    return 0;
}

I expect nc_put_att_int to report NC_ELATEFILL and leave the netcdf file untouched. Instead it reports unexpected NC_NOERR (0), and the netcdf file is changed:

3> gcc -g put-att1.c -I/opt/local/include -L/opt/local/lib -lnetcdf
4> ls -go testfile.nc 
-rw-r--r--  1   13542 Mar 28 18:45 testfile.nc
5> ./a.out
line 21 expecting NC_ELATEFILL but got 0
6> ls -go testfile.nc
-rw-r--r--  1   13810 Mar 28 19:28 testfile.nc

Sample netcdf file BEFORE modification. You can make your own test file by running this through ncgen:

netcdf testfile {
dimensions:
	dim = 10 ;
variables:
	int var(dim) ;
		var:_Storage = "contiguous" ;
		var:_Endianness = "little" ;

// global attributes:
		:_NCProperties = "version=1|netcdflibversion=4.4.1|hdf5libversion=1.8.17" ;
		:_SuperblockVersion = 0 ;
		:_IsNetcdf4 = 1 ;
		:_Format = "netCDF-4" ;
data:

 var = 11, 22, 33, 44, 55, _, _, _, _, _ ;
}

Test file AFTER running the put_var_att program. Note that five previous values at the start of "var" are erased, i.e. set to _FillValue:

netcdf testfile {
dimensions:
	dim = 10 ;
variables:
	int var(dim) ;
		var:_FillValue = 99 ;
		var:_Storage = "contiguous" ;
		var:_Endianness = "little" ;

// global attributes:
		:_NCProperties = "version=1|netcdflibversion=4.4.1|hdf5libversion=1.8.17" ;
		:_SuperblockVersion = 0 ;
		:_IsNetcdf4 = 1 ;
		:_Format = "netCDF-4" ;
data:

 var = _, _, _, _, _, _, _, _, _, _ ;
}
@DennisHeimbigner
Copy link
Collaborator

Why should this give an error?

@Dave-Allured
Copy link
Contributor Author

nc_put_att documentation, possible error return:

NC_ELATEFILL -- Fill values must be written while the file is still in initial define mode.

HDF5 FAQ:

"the fill value for a dataset cannot be modified once the dataset has been created"
https://support.hdfgroup.org/HDF5/hdf5-quest.html#pfillvalue

@DennisHeimbigner
Copy link
Collaborator

I see I mis-interpreted ELATEFILL.
;

@wkliao
Copy link
Contributor

wkliao commented Mar 29, 2017

Wow! I did not expect the erasure of previous written values (the first 5 elements).

@WardF
Copy link
Member

WardF commented Mar 29, 2017

Good to know, and this clarifies the speculation on the previous patch. It appears that we do indeed want to interpret this as the initial define mode.

@WardF WardF added this to the 4.4.2 milestone Mar 29, 2017
@DennisHeimbigner
Copy link
Collaborator

Does this issue relate to pr's 383, 387, and 389?

@WardF
Copy link
Member

WardF commented Mar 29, 2017

I believe that it does; I'm reviewing those now that we have this additional info, and am incorporating the tests that @wkliao has provided.

@Dave-Allured
Copy link
Contributor Author

You can mark this issue as low priority, as far as I am concerned. I stumbled on this behavior while I was curious about related issue #384. It is not a problem for my own applications.

@WardF WardF modified the milestones: 4.5.1, 4.5 May 24, 2017
@WardF WardF closed this as completed Jun 7, 2017
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this issue May 16, 2018
Upstream changes:
## 4.6.1 - March 15, 2018

* [Bug Fix] Corrected an issue which could result in a dap4 failure. See [Github #888](Unidata/netcdf-c#888) for more information.
* [Bug Fix][Enhancement] Allow `nccopy` to control output filter suppresion.  See [Github #894](Unidata/netcdf-c#894) for more information.
* [Enhancement] Reverted some new behaviors that, while in line with the netCDF specification, broke existing workflows.  See [Github #843](Unidata/netcdf-c#843) for more information.
* [Bug Fix] Improved support for CRT builds with Visual Studio, improves zlib detection in hdf5 library. See [Github #853](Unidata/netcdf-c#853) for more information.
* [Enhancement][Internal] Moved HDF4 into a distinct dispatch layer. See [Github #849](Unidata/netcdf-c#849) for more information.

## 4.6.0 - January 24, 2018
* [Enhancement] Full support for using HDF5 dynamic filters, both for reading and writing. See the file docs/filters.md.
* [Enhancement] Added an option to enable strict null-byte padding for headers; this padding was specified in the spec but was not enforced.  Enabling this option will allow you to check your files, as it will return an E_NULLPAD error.  It is possible for these files to have been written by older versions of libnetcdf.  There is no effective problem caused by this lack of null padding, so enabling these options is informational only.  The options for `configure` and `cmake` are `--enable-strict-null-byte-header-padding` and `-DENABLE_STRICT_NULL_BYTE_HEADER_PADDING`, respectively.  See [Github #657](Unidata/netcdf-c#657) for more information.
* [Enhancement] Reverted behavior/handling of out-of-range attribute values to pre-4.5.0 default. See [Github #512](Unidata/netcdf-c#512) for more information.
* [Bug] Fixed error in tst_parallel2.c. See [Github #545](Unidata/netcdf-c#545) for more information.
* [Bug] Fixed handling of corrupt files + proper offset handling for hdf5 files. See [Github #552](Unidata/netcdf-c#552) for more information.
* [Bug] Corrected a memory overflow in `tst_h_dimscales`, see [Github #511](Unidata/netcdf-c#511), [Github #505](Unidata/netcdf-c#505), [Github #363](Unidata/netcdf-c#363) and [Github #244](Unidata/netcdf-c#244) for more information.

## 4.5.0 - October 20, 2017

* Corrected an issue which could potential result in a hang while using parallel file I/O. See [Github #449](Unidata/netcdf-c#449) for more information.
* Addressed an issue with `ncdump` not properly handling dates on a 366 day calendar. See [GitHub #359](Unidata/netcdf-c#359) for more information.

### 4.5.0-rc3 - September 29, 2017

* [Update] Due to ongoing issues, native CDF5 support has been disabled by **default**.  You can use the options mentioned below (`--enable-cdf5` or `-DENABLE_CDF5=TRUE` for `configure` or `cmake`, respectively).  Just be aware that for the time being, Reading/Writing CDF5 files on 32-bit platforms may result in unexpected behavior when using extremely large variables.  For 32-bit platforms it is best to continue using `NC_FORMAT_64BIT_OFFSET`.
* [Bug] Corrected an issue where older versions of curl might fail. See [GitHub #487](Unidata/netcdf-c#487) for more information.
* [Enhancement] Added options to enable/disable `CDF5` support at configure time for autotools and cmake-based builds.  The options are `--enable/disable-cdf5` and `ENABLE_CDF5`, respectively.  See [Github #484](Unidata/netcdf-c#484) for more information.
* [Bug Fix] Corrected an issue when subsetting a netcdf3 file via `nccopy -v/-V`. See [Github #425](Unidata/netcdf-c#425) and [Github #463](Unidata/netcdf-c#463) for more information.
* [Bug Fix] Corrected `--has-dap` and `--has-dap4` output for cmake-based builds. See [GitHub #473](Unidata/netcdf-c#473) for more information.
* [Bug Fix] Corrected an issue where `NC_64BIT_DATA` files were being read incorrectly by ncdump, despite the data having been written correctly.  See [GitHub #457](Unidata/netcdf-c#457) for more information.
* [Bug Fix] Corrected a potential stack buffer overflow.  See [GitHub #450](Unidata/netcdf-c#450) for more information.

### 4.5.0-rc2 - August 7, 2017

* [Bug Fix] Addressed an issue with how cmake was implementing large file support on 32-bit systems. See [GitHub #385](Unidata/netcdf-c#385) for more information.
* [Bug Fix] Addressed an issue where ncgen would not respect keyword case. See [GitHub #310](Unidata/netcdf-c#310) for more information.

### 4.5.0-rc1 - June 5, 2017

* [Enhancement] DAP4 is now included. Since dap2 is the default for urls, dap4 must be specified by
(1) using "dap4:" as the url protocol, or
(2) appending "#protocol=dap4" to the end of the url, or
(3) appending "#dap4" to the end of the url
Note that dap4 is enabled by default but remote-testing is
disbled until the testserver situation is resolved.
* [Enhancement] The remote testing server can now be specified with the `--with-testserver` option to ./configure.
* [Enhancement] Modified netCDF4 to use ASCII for NC_CHAR.  See [Github Pull request #316](Unidata/netcdf-c#316) for more information.
* [Bug Fix] Corrected an error with how dimsizes might be read. See [Github #410](Unidata/netcdf-c#410) for more information.
* [Bug Fix] Corrected an issue where 'make check' would fail if 'make' or 'make all' had not run first.  See [Github #339](Unidata/netcdf-c#339) for more information.
* [Bug Fix] Corrected an issue on Windows with Large file tests. See [Github #385](Unidata/netcdf-c#385]) for more information.
* [Bug Fix] Corrected an issue with diskless file access, see [Pull Request #400](Unidata/netcdf-c#400) and [Pull Request #403](Unidata/netcdf-c#403) for more information.
* [Upgrade] The bash based test scripts have been upgraded to use a common test_common.sh include file that isolates build specific information.
* [Upgrade] The bash based test scripts have been upgraded to use a common test_common.sh include file that isolates build specific information.
* [Refactor] the oc2 library is no longer independent of the main netcdf-c library. For example, it now uses ncuri, nclist, and ncbytes instead of its homegrown equivalents.
* [Bug Fix] `NC_EGLOBAL` is now properly returned when attempting to set a global `_FillValue` attribute. See [GitHub #388](Unidata/netcdf-c#388) and [GitHub #389](Unidata/netcdf-c#389) for more information.
* [Bug Fix] Corrected an issue where data loss would occur when `_FillValue` was mistakenly allowed to be redefined.  See [Github #390](Unidata/netcdf-c#390), [GitHub #387](Unidata/netcdf-c#387) for more information.
* [Upgrade][Bug] Corrected an issue regarding how "orphaned" DAS attributes were handled. See [GitHub #376](Unidata/netcdf-c#376) for more information.
* [Upgrade] Update utf8proc.[ch] to use the version now maintained by the Julia Language project (https://github.com/JuliaLang/utf8proc/blob/master/LICENSE.md).
* [Bug] Addressed conversion problem with Windows sscanf.  This primarily affected some OPeNDAP URLs on Windows.  See [GitHub #365](Unidata/netcdf-c#365) and [GitHub #366](Unidata/netcdf-c#366) for more information.
* [Enhancement] Added support for HDF5 collective metadata operations when available. Patch submitted by Greg Sjaardema, see [Pull request #335](Unidata/netcdf-c#335) for more information.
* [Bug] Addressed a potential type punning issue. See [GitHub #351](Unidata/netcdf-c#351) for more information.
* [Bug] Addressed an issue where netCDF wouldn't build on Windows systems using MSVC 2012. See [GitHub #304](Unidata/netcdf-c#304) for more information.
* [Bug] Fixed an issue related to potential type punning, see [GitHub #344](Unidata/netcdf-c#344) for more information.
* [Enhancement] Incorporated an enhancement provided by Greg Sjaardema, which may improve read/write times for some complex files.  Basically, linked lists were replaced in some locations where it was safe to use an array/table.  See [Pull request #328](Unidata/netcdf-c#328) for more information.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants