From e426aee95b616778e4a8e484c3a56c691a2b9c52 Mon Sep 17 00:00:00 2001 From: Tim Burke Date: Fri, 21 Apr 2017 18:32:02 -0700 Subject: [PATCH] Un-inline get/set_metatdata_chksum Each was only really used in one place, they had some strange return types, and recent versions of clang on OS X would refuse to compile with erasurecode_helpers.c:531:26: error: taking address of packed member 'metadata_chksum' of class or structure 'fragment_header_s' may result in an unaligned pointer value [-Werror,-Waddress-of-packed-member] return (uint32_t *) &header->metadata_chksum; ^~~~~~~~~~~~~~~~~~~~~~~ We don't really *care* about the pointer; we just want the value! Change-Id: I8a5e42312948a75f5dd8b23b6f5ccfa7bd22eb1d --- include/erasurecode/erasurecode_helpers_ext.h | 2 - src/erasurecode.c | 17 +++++--- src/erasurecode_helpers.c | 43 ------------------- src/erasurecode_postprocessing.c | 13 +++++- test/liberasurecode_test.c | 4 -- 5 files changed, 22 insertions(+), 57 deletions(-) diff --git a/include/erasurecode/erasurecode_helpers_ext.h b/include/erasurecode/erasurecode_helpers_ext.h index 740d7db..8041a57 100644 --- a/include/erasurecode/erasurecode_helpers_ext.h +++ b/include/erasurecode/erasurecode_helpers_ext.h @@ -72,8 +72,6 @@ int set_backend_id(char *buf, ec_backend_id_t id); int get_backend_id(char *buf, ec_backend_id_t *id); int set_backend_version(char *buf, uint32_t version); int get_backend_version(char *buf, uint32_t *version); -int set_metadata_chksum(char *buf); -uint32_t *get_metadata_chksum(char *buf); int is_invalid_fragment_header(fragment_header_t *header); /* ==~=*=~==~=*=~==~=*=~==~=*=~==~=*=~==~=*=~==~=*=~==~=*=~==~=*=~==~=*=~== */ diff --git a/src/erasurecode.c b/src/erasurecode.c index 20da457..59e6d60 100644 --- a/src/erasurecode.c +++ b/src/erasurecode.c @@ -26,8 +26,8 @@ * vi: set noai tw=79 ts=4 sw=4: */ +#include #include -#include "assert.h" #include "list.h" #include "erasurecode.h" #include "erasurecode_backend.h" @@ -1093,7 +1093,7 @@ out: int is_invalid_fragment_header(fragment_header_t *header) { - uint32_t *stored_csum = NULL, csum = 0; + uint32_t csum = 0; assert (NULL != header); if (header->libec_version == 0) /* libec_version must be bigger than 0 */ @@ -1101,17 +1101,20 @@ int is_invalid_fragment_header(fragment_header_t *header) if (header->libec_version < _VERSION(1,2,0)) /* no metadata checksum support */ return 0; - stored_csum = get_metadata_chksum((char *) header); - if (NULL == stored_csum) - return 1; /* can't verify, possibly crc32 call error */ + + if (header->magic != LIBERASURECODE_FRAG_HEADER_MAGIC) { + log_error("Invalid fragment header (get meta chksum)!"); + return 1; + } + csum = crc32(0, (unsigned char *) &header->meta, sizeof(fragment_metadata_t)); - if (*stored_csum == csum) { + if (header->metadata_chksum == csum) { return 0; } // Else, try again with our "alternative" crc32; see // https://bugs.launchpad.net/liberasurecode/+bug/1666320 csum = liberasurecode_crc32_alt(0, &header->meta, sizeof(fragment_metadata_t)); - return (*stored_csum != csum); + return (header->metadata_chksum != csum); } int liberasurecode_verify_fragment_metadata(ec_backend_t be, diff --git a/src/erasurecode_helpers.c b/src/erasurecode_helpers.c index 4a49786..1494587 100644 --- a/src/erasurecode_helpers.c +++ b/src/erasurecode_helpers.c @@ -501,46 +501,3 @@ inline uint32_t* get_chksum(char *buf) } /* ==~=*=~==~=*=~==~=*=~==~=*=~==~=*=~==~=*=~==~=*=~==~=*=~==~=*=~==~=*=~== */ - -#if LIBERASURECODE_VERSION >= _VERSION(1,2,0) -inline int set_metadata_chksum(char *buf) -{ - fragment_header_t* header = (fragment_header_t*) buf; - - assert(NULL != header); - if (header->magic != LIBERASURECODE_FRAG_HEADER_MAGIC) { - log_error("Invalid fragment header (set meta chksum)!\n"); - return -1; - } - - header->metadata_chksum = crc32(0, (unsigned char *) &header->meta, - sizeof(fragment_metadata_t)); - return 0; -} - -inline uint32_t* get_metadata_chksum(char *buf) -{ - fragment_header_t* header = (fragment_header_t*) buf; - - assert(NULL != header); - if (header->magic != LIBERASURECODE_FRAG_HEADER_MAGIC) { - log_error("Invalid fragment header (get meta chksum)!"); - return NULL; - } - - return (uint32_t *) &header->metadata_chksum; -} -#else -inline int set_metadata_chksum(char *buf) -{ - return 0; -} - -inline uint32_t* get_metadata_chksum(char *buf) -{ - return (uint32_t *) 0; -} -#endif - -/* ==~=*=~==~=*=~==~=*=~==~=*=~==~=*=~==~=*=~==~=*=~==~=*=~==~=*=~==~=*=~== */ - diff --git a/src/erasurecode_postprocessing.c b/src/erasurecode_postprocessing.c index 7d121d4..07c08f5 100644 --- a/src/erasurecode_postprocessing.c +++ b/src/erasurecode_postprocessing.c @@ -26,9 +26,11 @@ * vi: set noai tw=79 ts=4 sw=4: */ +#include #include "erasurecode_backend.h" #include "erasurecode_helpers.h" #include "erasurecode_helpers_ext.h" +#include "erasurecode_log.h" #include "erasurecode_stdinc.h" void add_fragment_metadata(ec_backend_t be, char *fragment, @@ -49,7 +51,16 @@ void add_fragment_metadata(ec_backend_t be, char *fragment, if (add_chksum) { set_checksum(ct, fragment, blocksize); } - set_metadata_chksum(fragment); + + fragment_header_t* header = (fragment_header_t*) fragment; + + if (header->magic != LIBERASURECODE_FRAG_HEADER_MAGIC) { + log_error("Invalid fragment header (add fragment metadata)!\n"); + return; + } + + header->metadata_chksum = crc32(0, (unsigned char *) &header->meta, + sizeof(fragment_metadata_t)); } int finalize_fragments_after_encode(ec_backend_t instance, diff --git a/test/liberasurecode_test.c b/test/liberasurecode_test.c index 68c1c13..e62df94 100644 --- a/test/liberasurecode_test.c +++ b/test/liberasurecode_test.c @@ -1030,15 +1030,11 @@ static void encode_decode_test_impl(const ec_backend_id_t be_id, int cmp_size = -1; char *data_ptr = NULL; char *frag = NULL; - uint32_t *mcksum = NULL; frag = (i < args->k) ? encoded_data[i] : encoded_parity[i - args->k]; assert(frag != NULL); fragment_header_t *header = (fragment_header_t*)frag; assert(header != NULL); - mcksum = get_metadata_chksum(frag); - assert(mcksum != NULL); - assert(header->metadata_chksum == *mcksum); fragment_metadata_t metadata = header->meta; assert(metadata.idx == i);