Repository navigation
Conversation
NML's CMS_XDR_UPDATER (libnml/cms/cms_xup.cc) serializes through the system RPC/XDR implementation, but the later usability probe calls get_myaddress, which NML does not use. Probe the fixed-width integer filters, xdrmem_create and stream position and destroy macros that cms_xup.cc already uses. This is expected to be neutral on working Linux configurations and diagnoses missing declarations or symbols in config.log before building NML. The probe only compiles and links; char, bytes, vector, float and double filters are not covered. Calls use parenthesized names so an undeclared function is an error even where older C compilers would implicitly declare a plain call. Stream operation macros remain unparenthesized so they can expand. The earlier rpc/rpc.h and libtirpc discovery is unchanged, including its get_myaddress library-symbol check.
| AC_MSG_CHECKING([for usable fixed-width RPC/XDR routines]) | ||
| AC_LINK_IFELSE([AC_LANG_PROGRAM([[ | ||
| #include <rpc/rpc.h> | ||
| #include "libnml/cms/nml_xdr_compat.h" |
There was a problem hiding this comment.
Wouldn't this fail because config.h is not included?
This looks like a chicken-and-egg problem because config.h is generated by configure but you need it here for a test in configure. Besides, including from the source tree is not a "nice" thing to do.
There was a problem hiding this comment.
Thank you for checking! It builds on macOS and Linux, and I admittedly like including that header file from the source tree instead of some code duplication.
Codex:
The test does not require the generated config.h: the preceding AC_CHECK_DECLS calls populate confdefs.h, whose definitions Autoconf automatically prepends to the test program before the include.
I checked this in a fresh directory with config.h absent through the test; it passes, and the header is generated afterward.
The shared header was intended to test the same compatibility code NML uses. I understand your concern about that coupling. We could instead probe the native XDR routines and required fallback primitives through <rpc/rpc.h>, leaving the compatibility header solely in NML. The diagnostic mentioning config.h should also be clarified.
I would change only the error text in nml_xdr_compat.h:
-#error "include config.h before nml_xdr_compat.h"
+#error "XDR declaration results are missing (HAVE_DECL_XDR_*)"
This names the actual requirement without prescribing a particular file. The existing header comment already explains that these results come from config.h during normal compilation or confdefs.h during configure.
The guard stays unchanged: values of 0 are valid detection results; it rejects only undefined macros. This would be an editorial amendment to the second commit.
smoe again: Would that help? Maybe both messages together would be perfect?
There was a problem hiding this comment.
But, when you have checked, which you did, then you can infer that you need the polyfill and do not need to include the header to add the polyfill from the header.
There was a problem hiding this comment.
You have point in that configure can detect which fallback is needed without compiling the fallback itself.
I would revise patch 2 only:
- Keep AC_CHECK_DECLS.
- Make the link test include only <rpc/rpc.h> and <stdint.h>.
- Check either the declared native function or the underlying primitive needed by its fallback.
- Keep the compatibility header solely in NML, without copying its adapter functions into configure.
- Update comments that currently describe the header being used during configure.
The link checks should remain: knowing that a declaration is missing does not prove that its replacement primitive is available and linkable. Patch 1 can remain unchanged.
Some system RPC/XDR implementations do not declare every fixed-width
filter CMS_XDR_UPDATER uses. The tested macOS SDK omits xdr_int8_t,
xdr_uint8_t and xdr_uint{16,32,64}_t, while providing the corresponding
unsigned xdr_u_int{16,32,64}_t primitives.
Check the five names with AC_CHECK_DECLS. For names not declared,
libnml/cms/nml_xdr_compat.h maps unsigned 16/32/64-bit routines to their
xdr_u_intN_t equivalents, with the same 4/8-byte encodings. The signed and
unsigned 8-bit adapters send one 32-bit XDR word, matching glibc/libtirpc:
decoding narrows without a range check, leaves the destination unchanged
on failure, and XDR_FREE is a no-op.
Declared native routines are always used. Where all five names are
declared, the header only includes rpc/rpc.h and stdint.h. config.h gains
five HAVE_DECL_* results, which cms_xup.cc includes before the header.
The configure link test includes only system headers. It checks native
routines when declared and the primitives needed by the selected fallbacks
otherwise; the compatibility header is used only when compiling NML.
Mark the new header LGPL Version 2, matching the neighboring CMS files.
5991af1 to
f208d24
Compare
|
|
||
| AC_MSG_CHECKING([for usability of rpc/rpc.h]) | ||
| # The compatibility header fills gaps in these fixed-width names. | ||
| AC_CHECK_DECLS([xdr_int8_t, xdr_uint8_t, xdr_uint16_t, xdr_uint32_t, xdr_uint64_t], |
There was a problem hiding this comment.
There is an assumption that xdr_int16_t, xdr_int32_t and xdr_int64_t exist unconditionally, while the unsigned versions is tested for. But not for the 8-bit version.
What are the guarantees that, for example, xdr_int64_t exists. The 64-bit version is also likely to be missing with old lib versions.
There was a problem hiding this comment.
Here I am not with you. There is no assumption that every RPC library provides these functions. AC_CHECK_DECLS selects the available adaptations; the following AC_LINK_IFELSE requires the signed 16-, 32- and 64-bit routines. An older library missing one will fail configure. NML already uses these routines unconditionally. Will add a respective comment.
Include config.h inside nml_xdr_compat.h and include that header first in cms_xup.cc, preserving configuration before system-header processing. Use a container-prefixed include guard and mark this new header LGPL version 2 or later. Clarify that declaration checks select compatibility replacements, while the signed 16/32/64-bit primitives remain required by the link check. Adapter behavior and the RPC/TI-RPC discovery fallback are unchanged. Validation: isolated native macOS configure; C11 and C++20 wire fixtures (176 assertions each), with no caller-side config.h and repeated header inclusion; configuration-before-RPC include traces; 32 synthetic provider selections and 24 missing-declaration/definition controls. No project targets built. Native Linux validation was not repeated.
Hello,
Some (as I find) good news: My MacBook could eventually build some of our binaries. We are about four smaller patches away from it. All dependencies are installable via https://brew.sh. Changes are almost exclusively to the Makefiles or (like here) to the configuration.
I admit that I had used Codex to generate these changes. But I have also looked through them and think that they are good. I hope that you embrace these additions, less because of my Mac, more so because of other variants of Linux and why not also prepare for BSD.
NML’s XDR updater is bundled with LinuxCNC, but its serialization routines
come from the system RPC/XDR implementation. The later configure usability
check currently tests
get_myaddress, which NML does not call, rather thanthe fixed-width XDR interfaces it needs.
This series makes two focused changes:
by NML, giving an earlier diagnostic for missing declarations or symbols.
exist but the required fixed-width names are missing, as on the tested
macOS SDK.
Declared native routines remain preferred. Signedness and wire encodings
are unchanged. The existing RPC header discovery and TI-RPC fallback,
including its
get_myaddresslibrary-symbol check, remain unchanged.