CMake: fix XDR to be an external dependency #774
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fix build name when using dynamic plugins
Mercury configured with -DMERCURY_USE_XDR=ON fails to pass the unit tests. The main issue is that mercury was not properly accounting for the data padding that xdr uses. When encoding, xdr pads all data added to its buffer to the next multiple of BYTES_PER_XDR_UNIT (4 bytes, see /usr/include/rpc/xdr.h) and provides a macro RNDUP(s) to round "s" up to the next multiple of BYTES_PER_XDR_UNIT. The native mercury code (when not using xdr) does not pad data. When mercury is compiled to use xdr it does not account for the padding xdr uses and gets out of sync with xdr. Given an hg_proc_t "p" we need to make sure that hg_proc_get_size_used(p) is always equal to xdr_getpos(hg_proc_get_xdr_ptr(p)) after encoding a chunk. To fix this we: - modify the xdr version of HG_PROC_TYPE() in mercury_proc.h to round up the size using RNDUP(). - modify the xdr version of HG_PROC_BYTES() to mercury_proc.h to round up the byte chunk size using RNDUP() Note that the mercury checksum in this case does not cover the padding (since the calls to HG_PROC_CHECKSUM_UPDATE() are on the orig data buffer, and that buffer is not padded. This is not an issue, since the padding is thrown away at decode time. In addition, we change the xdr call in HG_PROC_BYTES() from xdr_bytes() to xdr_opaque() which better matches the intent of HG_PROC_BYTES(). For reference: xdr_opaque(xdr, char *data, size) copies RNDUP(size) bytes (the data plus padding) to the buffer. xdr_bytes(xdr, char **data, u_int *size, UINT_MAX) copies both a uint32_t length and RNDUP(size) bytes (the data plus padding) the buffer. HG_PROC_BYTES() does not need xdr to encode the length in the buffer (matches the non-xdr case). Remove code in mercury.c that forces payload_size to be the entire buffer. I do not think xdr requires the entire buffer to be payload if mercury accounts for the BYTES_PER_XDR_UNIT padding. Unit tests attempt to verify the expected payload length by manually computing it and comparing it to what is received. Update the tests to handle BYTES_PER_XDR_UNIT padding when compiled with xdr enabled.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.