From: Mark Abraham Date: Tue, 19 Mar 2019 02:55:02 +0000 (+0100) Subject: Add support for serializing 32-bit integers X-Git-Url: http://biod.pnpi.spb.ru/gitweb/?a=commitdiff_plain;h=582fc1fb21435daee55c66968a626c0f8380ac85;p=alexxy%2Fgromacs.git Add support for serializing 32-bit integers Some file formats explicitly require this size, which we should support explicitly. Note that C++ requires that int be at least 16 bits. In practice on current systems it is always 32 bits, but being explicit might help future portability if any architecture would use 64-bit int. Also added some support for unsigned 32- and 64-bit integers where I saw it was missing and might be useful. Added some test coverage where it already exists. Refs #1877 Change-Id: I05efd7d4ba07b695fc4ed7c9d9cac8738873a3a5 --- diff --git a/src/gromacs/fileio/gmxfio_xdr.cpp b/src/gromacs/fileio/gmxfio_xdr.cpp index 9a40af48dd..350ae03245 100644 --- a/src/gromacs/fileio/gmxfio_xdr.cpp +++ b/src/gromacs/fileio/gmxfio_xdr.cpp @@ -51,14 +51,14 @@ /* Enumerated for data types in files */ enum { - eioREAL, eioFLOAT, eioDOUBLE, eioINT, eioINT64, + eioREAL, eioFLOAT, eioDOUBLE, eioINT, eioINT32, eioINT64, eioUCHAR, eioNUCHAR, eioUSHORT, eioRVEC, eioNRVEC, eioIVEC, eioSTRING, eioNR }; static const char *eioNames[eioNR] = { - "REAL", "FLOAT", "DOUBLE", "INT", "INT64", + "REAL", "FLOAT", "DOUBLE", "INT", "INT32", "INT64", "UCHAR", "NUCHAR", "USHORT", "RVEC", "NRVEC", "IVEC", "STRING" }; @@ -113,7 +113,8 @@ static gmx_bool do_xdr(t_fileio *fio, void *item, int nitem, int eio, float fvec[DIM]; double dvec[DIM]; int j, m, *iptr, idum; - int64_t sdum; + int32_t s32dum; + int64_t s64dum; real *ptr; unsigned short us; double d = 0; @@ -182,15 +183,26 @@ static gmx_bool do_xdr(t_fileio *fio, void *item, int nitem, int eio, *static_cast(item) = idum; } break; + case eioINT32: + if (item && !fio->bRead) + { + s32dum = *static_cast(item); + } + res = xdr_int32(fio->xdr, &s32dum); + if (item) + { + *static_cast(item) = s32dum; + } + break; case eioINT64: if (item && !fio->bRead) { - sdum = *static_cast(item); + s64dum = *static_cast(item); } - res = xdr_int64(fio->xdr, &sdum); + res = xdr_int64(fio->xdr, &s64dum); if (item) { - *static_cast(item) = sdum; + *static_cast(item) = s64dum; } break; case eioUCHAR: @@ -428,6 +440,16 @@ gmx_bool gmx_fio_doe_int(t_fileio *fio, int *item, return ret; } +gmx_bool gmx_fio_doe_int32(t_fileio *fio, int32_t *item, + const char *desc, const char *srcfile, int line) +{ + gmx_bool ret; + gmx_fio_lock(fio); + ret = do_xdr(fio, item, 1, eioINT32, desc, srcfile, line); + gmx_fio_unlock(fio); + return ret; +} + gmx_bool gmx_fio_doe_int64(t_fileio *fio, int64_t *item, const char *desc, const char *srcfile, int line) { @@ -700,6 +722,11 @@ void FileIOXdrSerializer::doInt(int *value) gmx_fio_do_int(fio_, *value); } +void FileIOXdrSerializer::doInt32(int32_t *value) +{ + gmx_fio_do_int32(fio_, *value); +} + void FileIOXdrSerializer::doInt64(int64_t *value) { gmx_fio_do_int64(fio_, *value); diff --git a/src/gromacs/fileio/gmxfio_xdr.h b/src/gromacs/fileio/gmxfio_xdr.h index 27811f3030..33d1308b06 100644 --- a/src/gromacs/fileio/gmxfio_xdr.h +++ b/src/gromacs/fileio/gmxfio_xdr.h @@ -67,6 +67,8 @@ gmx_bool gmx_fio_doe_gmx_bool(struct t_fileio *fio, gmx_bool *item, const char *desc, const char *srcfile, int line); gmx_bool gmx_fio_doe_int(struct t_fileio *fio, int *item, const char *desc, const char *srcfile, int line); +gmx_bool gmx_fio_doe_int32(struct t_fileio *fio, int32_t *item, + const char *desc, const char *srcfile, int line); gmx_bool gmx_fio_doe_int64(struct t_fileio *fio, int64_t *item, const char *desc, const char *srcfile, int line); gmx_bool gmx_fio_doe_uchar(struct t_fileio *fio, unsigned char *item, @@ -91,6 +93,9 @@ gmx_bool gmx_fio_ndoe_gmx_bool(struct t_fileio *fio, gmx_bool *item, int n, const char *desc, const char *srcfile, int line); gmx_bool gmx_fio_ndoe_int(struct t_fileio *fio, int *item, int n, const char *desc, const char *srcfile, int line); +gmx_bool gmx_fio_ndoe_int32(struct t_fileio *fio, int32_t *item, int n, + const char *desc, const char *srcfile, + int line); gmx_bool gmx_fio_ndoe_int64(struct t_fileio *fio, int64_t *item, int n, const char *desc, const char *srcfile, int line); @@ -115,6 +120,7 @@ gmx_bool gmx_fio_ndoe_string(struct t_fileio *fio, char *item[], int n, #define gmx_fio_do_double(fio, item) gmx_fio_doe_double(fio, &(item), (#item), __FILE__, __LINE__) #define gmx_fio_do_gmx_bool(fio, item) gmx_fio_doe_gmx_bool(fio, &(item), (#item), __FILE__, __LINE__) #define gmx_fio_do_int(fio, item) gmx_fio_doe_int(fio, &(item), (#item), __FILE__, __LINE__) +#define gmx_fio_do_int32(fio, item) gmx_fio_doe_int32(fio, &(item), (#item), __FILE__, __LINE__) #define gmx_fio_do_int64(fio, item) gmx_fio_doe_int64(fio, &(item), (#item), __FILE__, __LINE__) #define gmx_fio_do_uchar(fio, item) gmx_fio_doe_uchar(fio, &(item), (#item), __FILE__, __LINE__) #define gmx_fio_do_ushort(fio, item) gmx_fio_doe_ushort(fio, &(item), (#item), __FILE__, __LINE__) @@ -128,6 +134,7 @@ gmx_bool gmx_fio_ndoe_string(struct t_fileio *fio, char *item[], int n, #define gmx_fio_ndo_double(fio, item, n) gmx_fio_ndoe_double(fio, item, n, (#item), __FILE__, __LINE__) #define gmx_fio_ndo_gmx_bool(fio, item, n) gmx_fio_ndoe_gmx_bool(fio, item, n, (#item), __FILE__, __LINE__) #define gmx_fio_ndo_int(fio, item, n) gmx_fio_ndoe_int(fio, item, n, (#item), __FILE__, __LINE__) +#define gmx_fio_ndo_int32(fio, item, n) gmx_fio_ndoe_int32(fio, item, n, (#item), __FILE__, __LINE__) #define gmx_fio_ndo_int64(fio, item, n) gmx_fio_ndoe_int64(fio, item, n, (#item), __FILE__, __LINE__) #define gmx_fio_ndo_uchar(fio, item, n) gmx_fio_ndoe_uchar(fio, item, n, (#item), __FILE__, __LINE__) #define gmx_fio_ndo_ushort(fio, item, n) gmx_fio_ndoe_ushort(fio, item, n, (#item), __FILE__, __LINE__) @@ -148,6 +155,7 @@ class FileIOXdrSerializer : public ISerializer void doBool(bool *value) override; void doUChar(unsigned char *value) override; void doInt(int *value) override; + void doInt32(int32_t *value) override; void doInt64(int64_t *value) override; void doFloat(float *value) override; void doDouble(double *value) override; diff --git a/src/gromacs/fileio/xdrd.cpp b/src/gromacs/fileio/xdrd.cpp index d657619931..912de023dd 100644 --- a/src/gromacs/fileio/xdrd.cpp +++ b/src/gromacs/fileio/xdrd.cpp @@ -3,7 +3,7 @@ * * Copyright (c) 1991-2000, University of Groningen, The Netherlands. * Copyright (c) 2001-2004, The GROMACS development team. - * Copyright (c) 2013,2014,2015,2018, by the GROMACS development team, led by + * Copyright (c) 2013,2014,2015,2018,2019, by the GROMACS development team, led by * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl, * and including many others, as listed in the AUTHORS file in the * top-level source directory and at http://www.gromacs.org. @@ -92,10 +92,27 @@ int xdr3drcoord(XDR *xdrs, real *fp, int *size, real *precision) #endif } -int xdr_int64(XDR *xdrs, int64_t *i) +int xdr_int32(XDR *xdrs, int32_t *i) { - /* This routine stores values compatible with xdr_int64_t */ + // Note that this implementation assumes that an int is at least + // 32 bits, which is not strictly required by the language, but + // good enough in practice on 32- or 64-bit systems. GROMACS + // requires 64-bit systems. + static_assert(sizeof(int) >= 4, "XDR handling assumes that an int32_t can be stored in an int"); + int temporary = static_cast(*i); + int ret = xdr_int(xdrs, &temporary); + *i = static_cast(temporary); + + return ret; +} +int xdr_int64(XDR *xdrs, int64_t *i) +{ + // Note that this implementation assumes that an int is at least + // 32 bits, which is not strictly required by the language, but + // good enough in practice on 32- or 64-bit systems. GROMACS + // requires 64-bit systems. + static_assert(2*sizeof(int) >= 8, "XDR handling assumes that an int64_t can be stored in two ints"); int imaj, imin; int ret; diff --git a/src/gromacs/fileio/xdrf.h b/src/gromacs/fileio/xdrf.h index 8aa2f15ead..dab6069031 100644 --- a/src/gromacs/fileio/xdrf.h +++ b/src/gromacs/fileio/xdrf.h @@ -3,7 +3,7 @@ * * Copyright (c) 1991-2000, University of Groningen, The Netherlands. * Copyright (c) 2001-2004, The GROMACS development team. - * Copyright (c) 2013,2014,2015,2018, by the GROMACS development team, led by + * Copyright (c) 2013,2014,2015,2018,2019, by the GROMACS development team, led by * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl, * and including many others, as listed in the AUTHORS file in the * top-level source directory and at http://www.gromacs.org. @@ -67,13 +67,11 @@ int xdr_real(XDR *xdrs, real *r); int xdr3drcoord(XDR *xdrs, real *fp, int *size, real *precision); +//! Read or write a int32_t value. +int xdr_int32(XDR *xdrs, int32_t *i); + +//! Read or write a int64_t value. int xdr_int64(XDR *xdrs, int64_t *i); -/* Read or write a int64_t value. - * When warn!=NULL a warning will be written to stderr - * when a value does not fit, - * the first line is: - * "WARNING during %s:", where warn is printed in %s. - */ int xdr_xtc_seek_time(real time, FILE *fp, XDR *xdrs, int natoms, gmx_bool bSeekForwardOnly); diff --git a/src/gromacs/utility/inmemoryserializer.cpp b/src/gromacs/utility/inmemoryserializer.cpp index 451a167ef8..2d98a8e70e 100644 --- a/src/gromacs/utility/inmemoryserializer.cpp +++ b/src/gromacs/utility/inmemoryserializer.cpp @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2016,2017,2018, by the GROMACS development team, led by + * Copyright (c) 2016,2017,2018,2019, by the GROMACS development team, led by * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl, * and including many others, as listed in the AUTHORS file in the * top-level source directory and at http://www.gromacs.org. @@ -128,6 +128,11 @@ void InMemorySerializer::doInt(int *value) impl_->doValue(*value); } +void InMemorySerializer::doInt32(int32_t *value) +{ + impl_->doValue(*value); +} + void InMemorySerializer::doInt64(int64_t *value) { impl_->doValue(*value); @@ -202,6 +207,11 @@ void InMemoryDeserializer::doInt(int *value) impl_->doValue(value); } +void InMemoryDeserializer::doInt32(int32_t *value) +{ + impl_->doValue(value); +} + void InMemoryDeserializer::doInt64(int64_t *value) { impl_->doValue(value); diff --git a/src/gromacs/utility/inmemoryserializer.h b/src/gromacs/utility/inmemoryserializer.h index a873cc9dd3..b5824f231f 100644 --- a/src/gromacs/utility/inmemoryserializer.h +++ b/src/gromacs/utility/inmemoryserializer.h @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2016,2017,2018, by the GROMACS development team, led by + * Copyright (c) 2016,2017,2018,2019, by the GROMACS development team, led by * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl, * and including many others, as listed in the AUTHORS file in the * top-level source directory and at http://www.gromacs.org. @@ -64,6 +64,7 @@ class InMemorySerializer : public ISerializer void doBool(bool *value) override; void doUChar(unsigned char *value) override; void doInt(int *value) override; + void doInt32(int32_t *value) override; void doInt64(int64_t *value) override; void doFloat(float *value) override; void doDouble(double *value) override; @@ -86,6 +87,7 @@ class InMemoryDeserializer : public ISerializer void doBool(bool *value) override; void doUChar(unsigned char *value) override; void doInt(int *value) override; + void doInt32(int32_t *value) override; void doInt64(int64_t *value) override; void doFloat(float *value) override; void doDouble(double *value) override; diff --git a/src/gromacs/utility/iserializer.h b/src/gromacs/utility/iserializer.h index f4692963a3..4d5aa185f5 100644 --- a/src/gromacs/utility/iserializer.h +++ b/src/gromacs/utility/iserializer.h @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2016,2017,2018, by the GROMACS development team, led by + * Copyright (c) 2016,2017,2018,2019, by the GROMACS development team, led by * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl, * and including many others, as listed in the AUTHORS file in the * top-level source directory and at http://www.gromacs.org. @@ -69,6 +69,7 @@ class ISerializer virtual void doBool(bool *value) = 0; virtual void doUChar(unsigned char *value) = 0; virtual void doInt(int *value) = 0; + virtual void doInt32(int32_t *value) = 0; virtual void doInt64(int64_t *value) = 0; virtual void doFloat(float *value) = 0; virtual void doDouble(double *value) = 0; diff --git a/src/gromacs/utility/tests/keyvaluetreeserializer.cpp b/src/gromacs/utility/tests/keyvaluetreeserializer.cpp index e93063f31c..6d8ec01e01 100644 --- a/src/gromacs/utility/tests/keyvaluetreeserializer.cpp +++ b/src/gromacs/utility/tests/keyvaluetreeserializer.cpp @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2016,2017,2018, by the GROMACS development team, led by + * Copyright (c) 2016,2017,2018,2019, by the GROMACS development team, led by * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl, * and including many others, as listed in the AUTHORS file in the * top-level source directory and at http://www.gromacs.org. @@ -70,6 +70,10 @@ class RefDataSerializer : public gmx::ISerializer { checker_.checkInteger(*value, nullptr); } + void doInt32(int32_t *value) override + { + checker_.checkInt32(*value, nullptr); + } void doInt64(int64_t *value) override { checker_.checkInt64(*value, nullptr); @@ -132,6 +136,8 @@ TEST_F(KeyValueTreeSerializerTest, EmptyTree) TEST_F(KeyValueTreeSerializerTest, SimpleObject) { builder_.rootObject().addValue("foo", 1); + builder_.rootObject().addValue("foo32", 1); + builder_.rootObject().addValue("foo64", 1); builder_.rootObject().addValue("bar", "a"); builder_.rootObject().addValue("f", 1.5); builder_.rootObject().addValue("d", 2.5); diff --git a/src/gromacs/utility/tests/refdata/KeyValueTreeSerializerTest_SimpleObject.xml b/src/gromacs/utility/tests/refdata/KeyValueTreeSerializerTest_SimpleObject.xml index 5e7a62f667..f9d0cdf1d9 100644 --- a/src/gromacs/utility/tests/refdata/KeyValueTreeSerializerTest_SimpleObject.xml +++ b/src/gromacs/utility/tests/refdata/KeyValueTreeSerializerTest_SimpleObject.xml @@ -3,15 +3,23 @@ 1 + 1 + 1 a 1.5 2.5 - 4 + 6 foo 105 1 + foo32 + 105 + 1 + foo64 + 108 + 1 bar 115 a diff --git a/src/testutils/refdata.cpp b/src/testutils/refdata.cpp index 1e22a71f7b..0482fdf45e 100644 --- a/src/testutils/refdata.cpp +++ b/src/testutils/refdata.cpp @@ -388,7 +388,11 @@ class TestReferenceChecker::Impl static const char * const cUCharNodeName; //! String constant for naming XML elements for integer values. static const char * const cIntegerNodeName; - //! String constant for naming XML elements for int64 values. + //! String constant for naming XML elements for int32 values. + static const char * const cInt32NodeName; + //! String constant for naming XML elements for unsigned int32 values. + static const char * const cUInt32NodeName; + //! String constant for naming XML elements for int32 values. static const char * const cInt64NodeName; //! String constant for naming XML elements for unsigned int64 values. static const char * const cUInt64NodeName; @@ -555,6 +559,8 @@ const char *const TestReferenceChecker::Impl::cBooleanNodeName = "Bool"; const char *const TestReferenceChecker::Impl::cStringNodeName = "String"; const char *const TestReferenceChecker::Impl::cUCharNodeName = "UChar"; const char *const TestReferenceChecker::Impl::cIntegerNodeName = "Int"; +const char *const TestReferenceChecker::Impl::cInt32NodeName = "Int32"; +const char *const TestReferenceChecker::Impl::cUInt32NodeName = "UInt32"; const char *const TestReferenceChecker::Impl::cInt64NodeName = "Int64"; const char *const TestReferenceChecker::Impl::cUInt64NodeName = "UInt64"; const char *const TestReferenceChecker::Impl::cRealNodeName = "Real"; @@ -904,6 +910,18 @@ void TestReferenceChecker::checkInteger(int value, const char *id) ExactStringChecker(formatString("%d", value)))); } +void TestReferenceChecker::checkInt32(int32_t value, const char *id) +{ + EXPECT_PLAIN(impl_->processItem(Impl::cInt32NodeName, id, + ExactStringChecker(formatString("%" PRId32, value)))); +} + +void TestReferenceChecker::checkUInt32(uint32_t value, const char *id) +{ + EXPECT_PLAIN(impl_->processItem(Impl::cUInt32NodeName, id, + ExactStringChecker(formatString("%" PRIu32, value)))); +} + void TestReferenceChecker::checkInt64(int64_t value, const char *id) { EXPECT_PLAIN(impl_->processItem(Impl::cInt64NodeName, id, @@ -986,10 +1004,22 @@ void TestReferenceChecker::checkAny(const Any &any, const char *id) { checkInteger(any.cast(), id); } + else if (any.isType()) + { + checkInt32(any.cast(), id); + } + else if (any.isType()) + { + checkInt32(any.cast(), id); + } else if (any.isType()) { checkInt64(any.cast(), id); } + else if (any.isType()) + { + checkInt64(any.cast(), id); + } else if (any.isType()) { checkFloat(any.cast(), id); @@ -1073,6 +1103,19 @@ int TestReferenceChecker::readInteger(const char *id) } +int32_t TestReferenceChecker::readInt32(const char *id) +{ + if (impl_->shouldIgnore()) + { + GMX_THROW(TestException("Trying to read from non-existent reference data value")); + } + int32_t value = 0; + EXPECT_PLAIN(impl_->processItem(Impl::cInt32NodeName, id, + ValueExtractor(&value))); + return value; +} + + int64_t TestReferenceChecker::readInt64(const char *id) { if (impl_->shouldIgnore()) diff --git a/src/testutils/refdata.h b/src/testutils/refdata.h index d6c04e448e..efd63e1881 100644 --- a/src/testutils/refdata.h +++ b/src/testutils/refdata.h @@ -354,6 +354,10 @@ class TestReferenceChecker void checkUChar(unsigned char value, const char *id); //! Check a single integer value. void checkInteger(int value, const char *id); + //! Check a single int32 value. + void checkInt32(int32_t value, const char *id); + //! Check a single uint32 value. + void checkUInt32(uint32_t value, const char *id); //! Check a single int64 value. void checkInt64(int64_t value, const char *id); //! Check a single uint64 value. @@ -396,6 +400,8 @@ class TestReferenceChecker unsigned char readUChar(const char *id); //! Reads an integer value. int readInteger(const char *id); + //! Reads a 32-bit integer value. + int32_t readInt32(const char *id); //! Reads a 64-bit integer value. int64_t readInt64(const char *id); //! Reads a float value. @@ -430,17 +436,17 @@ class TestReferenceChecker { checkString(value, id); } - //! Check a single integer value. + //! Check a single signed integer value void checkValue(int value, const char *id) { checkInteger(value, id); } - //! Check a single integer value. + //! Check a single signed integer value of width 64 bits. void checkValue(int64_t value, const char *id) { checkInt64(value, id); } - //! Check a single integer value. + //! Check a single unsigned integer value of width 64 bits. void checkValue(uint64_t value, const char *id) { checkUInt64(value, id); diff --git a/src/testutils/tests/refdata_tests.cpp b/src/testutils/tests/refdata_tests.cpp index c7f4e3758e..c6efbe2029 100644 --- a/src/testutils/tests/refdata_tests.cpp +++ b/src/testutils/tests/refdata_tests.cpp @@ -69,6 +69,8 @@ TEST(ReferenceDataTest, HandlesSimpleData) TestReferenceChecker checker(data.rootChecker()); checker.checkBoolean(true, "bool"); checker.checkInteger(1, "int"); + checker.checkInt32(1ULL<<12, "int32"); + checker.checkUInt32(1ULL<<12, "uint32"); checker.checkInt64(1ULL<<42, "int64"); checker.checkUInt64(1ULL<<42, "uint64"); checker.checkDouble(0.5, "real"); @@ -79,6 +81,8 @@ TEST(ReferenceDataTest, HandlesSimpleData) TestReferenceChecker checker(data.rootChecker()); checker.checkBoolean(true, "bool"); checker.checkInteger(1, "int"); + checker.checkInt32(1ULL<<12, "int32"); + checker.checkUInt32(1ULL<<12, "uint32"); checker.checkInt64(1ULL<<42, "int64"); checker.checkUInt64(1ULL<<42, "uint64"); checker.checkDouble(0.5, "real");