On 02/11/2013 07:07 PM, Eric Blake wrote: > We have several cases where we need to read endian-dependent > data regardless of host endianness; rather than open-coding > these call sites, it will be nicer to funnel things through > a macro. > > The virendian.h file can be expanded to add writer functions, > and/or 16-bit access patterns, if needed. Also, if we need > to turn things into a function to avoid multiple evaluations > of buf, that can be done later. But for now, a macro worked. > > * src/util/virendian.h: New file. > * src/Makefile.am (UTIL_SOURCES): Ship it. > * tests/virendiantest.c: New test. > * tests/Makefile.am (test_programs, virendiantest_SOURCES): Run > the test. > * .gitignore: Ignore built file. > --- > .gitignore | 1 + > src/Makefile.am | 1 + > src/util/virendian.h | 93 +++++++++++++++++++++++++++++++++++++++++++++ > tests/Makefile.am | 8 +++- > tests/virendiantest.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++ > 5 files changed, 203 insertions(+), 2 deletions(-) > create mode 100644 src/util/virendian.h > create mode 100644 tests/virendiantest.c > > diff --git a/.gitignore b/.gitignore > index 1670637..8afbf33 100644 > --- a/.gitignore > +++ b/.gitignore > @@ -177,6 +177,7 @@ > /tests/virbitmaptest > /tests/virbuftest > /tests/virdrivermoduletest > +/tests/virendiantest > /tests/virhashtest > /tests/virkeyfiletest > /tests/virlockspacetest > diff --git a/src/Makefile.am b/src/Makefile.am > index f6162df..d554aa1 100644 > --- a/src/Makefile.am > +++ b/src/Makefile.am > @@ -67,6 +67,7 @@ UTIL_SOURCES = \ > util/virdbus.c util/virdbus.h \ > util/virdnsmasq.c util/virdnsmasq.h \ > util/virebtables.c util/virebtables.h \ > + util/virendian.h \ > util/virerror.c util/virerror.h \ > util/virevent.c util/virevent.h \ > util/vireventpoll.c util/vireventpoll.h \ > diff --git a/src/util/virendian.h b/src/util/virendian.h > new file mode 100644 > index 0000000..eefe48c > --- /dev/null > +++ b/src/util/virendian.h > @@ -0,0 +1,93 @@ > +/* > + * virendian.h: aid for reading endian-specific data > + * > + * Copyright (C) 2013 Red Hat, Inc. > + * > + * This library is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2.1 of the License, or (at your option) any later version. > + * > + * This library is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with this library. If not, see > + * <http://www.gnu.org/licenses/>. > + * > + */ > + > +#ifndef __VIR_ENDIAN_H__ > +# define __VIR_ENDIAN_H__ > + > +# include "internal.h" > + > +/* The interfaces in this file are provided as macros for speed. */ > + > +/** > + * virReadBufInt64BE: > + * @buf: byte to start reading at (can be 'char*' or 'unsigned char*'); > + * evaluating buf must not have any side effects > + * > + * Read 8 bytes at BUF as a big-endian 64-bit number. Caller is > + * responsible to avoid reading beyond array bounds. > + */ > +# define virReadBufInt64BE(buf) \ > + (((uint64_t)(uint8_t)((buf)[0]) << 56) | \ > + ((uint64_t)(uint8_t)((buf)[1]) << 48) | \ > + ((uint64_t)(uint8_t)((buf)[2]) << 40) | \ > + ((uint64_t)(uint8_t)((buf)[3]) << 32) | \ > + ((uint64_t)(uint8_t)((buf)[4]) << 24) | \ > + ((uint64_t)(uint8_t)((buf)[5]) << 16) | \ > + ((uint64_t)(uint8_t)((buf)[6]) << 8) | \ > + (uint64_t)(uint8_t)((buf)[7])) > + > +/** > + * virReadBufInt64LE: > + * @buf: byte to start reading at (can be 'char*' or 'unsigned char*'); > + * evaluating buf must not have any side effects > + * > + * Read 8 bytes at BUF as a little-endian 64-bit number. Caller is > + * responsible to avoid reading beyond array bounds. > + */ > +# define virReadBufInt64LE(buf) \ > + ((uint64_t)(uint8_t)((buf)[0]) | \ > + ((uint64_t)(uint8_t)((buf)[1]) << 8) | \ > + ((uint64_t)(uint8_t)((buf)[2]) << 16) | \ > + ((uint64_t)(uint8_t)((buf)[3]) << 24) | \ > + ((uint64_t)(uint8_t)((buf)[4]) << 32) | \ > + ((uint64_t)(uint8_t)((buf)[5]) << 40) | \ > + ((uint64_t)(uint8_t)((buf)[6]) << 48) | \ > + ((uint64_t)(uint8_t)((buf)[7]) << 56)) > + > +/** > + * virReadBufInt32BE: > + * @buf: byte to start reading at (can be 'char*' or 'unsigned char*'); > + * evaluating buf must not have any side effects > + * > + * Read 4 bytes at BUF as a big-endian 32-bit number. Caller is > + * responsible to avoid reading beyond array bounds. > + */ > +# define virReadBufInt32BE(buf) \ > + (((uint32_t)(uint8_t)((buf)[0]) << 24) | \ > + ((uint32_t)(uint8_t)((buf)[1]) << 16) | \ > + ((uint32_t)(uint8_t)((buf)[2]) << 8) | \ > + (uint32_t)(uint8_t)((buf)[3])) > + > +/** > + * virReadBufInt32LE: > + * @buf: byte to start reading at (can be 'char*' or 'unsigned char*'); > + * evaluating buf must not have any side effects > + * > + * Read 4 bytes at BUF as a little-endian 32-bit number. Caller is > + * responsible to avoid reading beyond array bounds. > + */ > +# define virReadBufInt32LE(buf) \ > + ((uint32_t)(uint8_t)((buf)[0]) | \ > + ((uint32_t)(uint8_t)((buf)[1]) << 8) | \ > + ((uint32_t)(uint8_t)((buf)[2]) << 16) | \ > + ((uint32_t)(uint8_t)((buf)[3]) << 24)) > + > +#endif /* __VIR_ENDIAN_H__ */ Looking at byteswap.h.in I'm reminded of what I've seen in a previous job where we had lots of byteswapping to do. The macros will "isolate" each byte to move, eg: #define bswap_32(x) ((((x) & 0x000000FF) << 24) | \ (((x) & 0x0000FF00) << 8) | \ (((x) & 0x00FF0000) >> 8) | \ (((x) & 0xFF000000) >> 24)) Not sure if it's necessary in this case, but figured I'd ask. > diff --git a/tests/Makefile.am b/tests/Makefile.am > index 0194db2..7d0a88e 100644 > --- a/tests/Makefile.am > +++ b/tests/Makefile.am > @@ -1,6 +1,6 @@ > ## Process this file with automake to produce Makefile.in > > -## Copyright (C) 2005-2012 Red Hat, Inc. > +## Copyright (C) 2005-2013 Red Hat, Inc. > ## See COPYING.LIB for the License of this software > > SHELL = $(PREFERABLY_POSIX_SHELL) > @@ -95,7 +95,7 @@ test_programs = virshtest sockettest \ > utiltest shunloadtest \ > virtimetest viruritest virkeyfiletest \ > virauthconfigtest \ > - virbitmaptest \ > + virbitmaptest virendiantest \ > virlockspacetest \ > virstringtest \ > virportallocatortest \ > @@ -649,6 +649,10 @@ virbitmaptest_SOURCES = \ > virbitmaptest.c testutils.h testutils.c > virbitmaptest_LDADD = $(LDADDS) > > +virendiantest_SOURCES = \ > + virendiantest.c testutils.h testutils.c > +virendiantest_LDADD = $(LDADDS) > + > jsontest_SOURCES = \ > jsontest.c testutils.h testutils.c > jsontest_LDADD = $(LDADDS) > diff --git a/tests/virendiantest.c b/tests/virendiantest.c > new file mode 100644 > index 0000000..386ef33 > --- /dev/null > +++ b/tests/virendiantest.c > @@ -0,0 +1,102 @@ > +/* > + * Copyright (C) 2013 Red Hat, Inc. > + * > + * This library is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2.1 of the License, or (at your option) any later version. > + * > + * This library is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with this library. If not, see > + * <http://www.gnu.org/licenses/>. > + * > + */ > + > +#include <config.h> > + > +#include "testutils.h" > + > +#include "virendian.h" > + > +static int > +test1(const void *data ATTRIBUTE_UNUSED) > +{ > + /* Regular char should work, even if signed, and even with > + * unaligned access. */ > + char array[] = { 1, 2, 3, 4, 5, 6, 7, 8, > + 0x89, 0x8a, 0x8b, 0x8c, 0x8d }; > + int ret = -1; > + > + if (virReadBufInt64BE(array) != 0x0102030405060708ULL) > + goto cleanup; > + if (virReadBufInt64BE(array + 5) != 0x060708898a8b8c8dULL) > + goto cleanup; > + if (virReadBufInt64LE(array) != 0x0807060504030201ULL) > + goto cleanup; > + if (virReadBufInt64LE(array + 5) != 0x8d8c8b8a89080706ULL) > + goto cleanup; > + > + if (virReadBufInt32BE(array) != 0x01020304U) > + goto cleanup; > + if (virReadBufInt32BE(array + 8) != 0x898a8b8cU) > + goto cleanup; > + if (virReadBufInt32LE(array) != 0x04030201U) > + goto cleanup; > + if (virReadBufInt32LE(array + 8) != 0x8c8b8a89U) > + goto cleanup; > + > + ret = 0; > +cleanup: > + return ret; > +} > + > +static int > +test2(const void *data ATTRIBUTE_UNUSED) > +{ > + /* Unsigned char should work without cast, even if unaligned access. */ > + unsigned char array[] = { 1, 2, 3, 4, 5, 6, 7, 8, > + 0x89, 0x8a, 0x8b, 0x8c, 0x8d }; > + int ret = -1; > + > + if (virReadBufInt64BE(array) != 0x0102030405060708ULL) > + goto cleanup; > + if (virReadBufInt64BE(array + 5) != 0x060708898a8b8c8dULL) > + goto cleanup; > + if (virReadBufInt64LE(array) != 0x0807060504030201ULL) > + goto cleanup; > + if (virReadBufInt64LE(array + 5) != 0x8d8c8b8a89080706ULL) > + goto cleanup; > + > + if (virReadBufInt32BE(array) != 0x01020304U) > + goto cleanup; > + if (virReadBufInt32BE(array + 8) != 0x898a8b8cU) > + goto cleanup; > + if (virReadBufInt32LE(array) != 0x04030201U) > + goto cleanup; > + if (virReadBufInt32LE(array + 8) != 0x8c8b8a89U) > + goto cleanup; > + > + ret = 0; > +cleanup: > + return ret; > +} > + > +static int > +mymain(void) > +{ > + int ret = 0; > + > + if (virtTestRun("test1", 1, test1, NULL) < 0) > + ret = -1; > + if (virtTestRun("test2", 1, test2, NULL) < 0) > + ret = -1; > + > + return ret; > +} > + > +VIRT_TEST_MAIN(mymain) > ACK -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list