-
Notifications
You must be signed in to change notification settings - Fork 301
Mac xdr workaround #4390
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
Mac xdr workaround #4390
Conversation
Refs libMesh#4387 Currently on some Mac builds we're seeing: ../src/utils/xdr_cxx.C:565:12: error: cast from 'int (*)(XDR *, int *)' (aka 'int (*)(__rpc_xdr *, int *)') to 'xdrproc_t' (aka 'int (*)(__rpc_xdr *, void *, unsigned int)') converts to incompatible function type [-Werror,-Wcast-function-type-mismatch] 565 | return (xdrproc_t)(xdr_int); I'd think "Are the XDR procs of type xdrproc?" would be an "Is the Pope Catholic?" sort of question, but here we are.
| // builds when trying to pass XDR's own functions to xdrproc_t | ||
| // arguments - apparently because the xdr_foo functions now only take | ||
| // two arguments? We'll add some shims to convert them. | ||
| #ifdef __APPLE__ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this workaround also apply to previous versions of MacOS where we never had this problem in the first place? In other words, is checking __APPLE__ sufficient here, or do you have to check for a specific version as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this workaround also apply to previous versions of MacOS where we never had this problem in the first place?
In theory it even applies to non-MacOS XDR implementations, we just restrict it to __APPLE__ because that extra function call is unnecessary on a good XDR implementation.
In practice ... is there just no such thing as a good XDR implementation??? I tried changing #ifdef __APPLE__ to #if 1, and it turns out that on my Ubuntu system I can't actually call xdr_longlong_t(x, v) with long long * v, because xdr_longlong_t takes a quad_t * argument instead, quad_t on my system is defined as long, and compilers refuse to silently convert between a pointer to one built-in type and a pointer to a binary-identical built-in type if those types have different names.
Why does my XDR library not have an xdr_quad_t(x,v) that takes a quad_t * argument, while letting xdr_longlong_t(x,v) take a long long * argument? Wouldn't that make sense, since long long is in the C++ standard, and quad_t isn't even in all of POSIX, and also because the strings longlong_t and long long seem so related that their similarity shouldn't be just coincidence? Some searching indicates that that API dates back at least to "Initial commit of libtirpc 0.1.7" in 2007, which is powerful evidence against my "Covid does subtle long-term damage to the human brain" and "Social media does subtle long-term damage to the human brain" theories, but my theory of "hominid evolution takes a million years, so at most we're 1% smarter than the stupidest apes capable of creating civilization" remains undefeated.
I believe I can make this into something robust using a variadic template trick. This belief is probably just evidence that I'm stupid too. Stay tuned...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I pushed the hopefully-more-robust option (at least it works on Linux TIRPC too now, so hopefully it won't regress on Mac).
Looking at the previous CI tests, though, it seems like we've still got some non-XDR issues to work out too before we'll be passing our Mac tests again.
|
Job Coverage, step Generate coverage on fdc5431 wanted to post the following: Coverage
Warnings
This comment will be updated on new commits. |
||||||||||||||||||||||||||
With these changes, the shim code works for my system's broken-in-yet-a-different-way TIRPC XDR implementation too.
|
I can at least replicate the MetaPhysicaL error now |
|
"Test mac" is passing, everything else is built; rather than wait to see if a syntax change affected dbg mode unit tests, I'm going to merge this and see if we can finally get a devel->master update. |
Refs #4387
Currently on some Mac builds we're seeing:
../src/utils/xdr_cxx.C:565:12: error: cast from 'int (*)(XDR *, int )' (aka 'int ()(__rpc_xdr *, int )') to 'xdrproc_t' (aka 'int ()(__rpc_xdr *, void *, unsigned int)') converts to incompatible function type [-Werror,-Wcast-function-type-mismatch]
565 | return (xdrproc_t)(xdr_int);
I'd think "Are the XDR procs of type xdrproc?" would be an "Is the Pope Catholic?" sort of question, but here we are.
Also, at least on my system while testing a fix for the above, I encountered the fact that my MacOSX SDK's rpc.h can define
xdr_longas takingint*rather thanlong*, so I added a workaround for that case.This is passing my initial tests; hopefully CI will catch anything I missed.