[should I move this to tech-toolchain?] chris...@zoulas.com wrote:
> | > | > I think that vasnprintf and asnprintf are not used by anything in > | > | > heimdal and can safely be removed. Combined with the winsize fix, > | > | > this fixes the cygwin problems with minimal changes. I am trying a > | > | > build now. > | > | > | > | Note roken.h includes <resolv.h> and <arpa/nameser.h> that don't > | > | exist on Cygwin so we had to handle it in src/tools/compat/configure. > | > | (toolchain/29032) > | > > | > Which is fine; I'd rather have one place to keep roken.h and deal with > | > portability in compat, rather than 2. > | > | But src/include/heimdal/roken.h is a generated file for NetBSD > | with unusual method. > | > | We might be able to generate roken.h for tools host from > | src/crypto/dist/heimdal/lib/roken/roken.h.in using > | src/crypto/dist/heimdal/lib/roken/roken.awk as defined > | in src/crypto/dist/heimdal/lib/roken/Makefile.am: > | --- > | roken.h: make-roken$(EXEEXT) > | @./make-roken$(EXEEXT) > tmp.h ;\ > | if [ -f roken.h ] && cmp -s tmp.h roken.h ; then rm -f tmp.h ; \ > | else rm -f roken.h; mv tmp.h roken.h; fi > | > | make-roken.c: roken.h.in roken.awk > | $(AWK) -f $(srcdir)/roken.awk $(srcdir)/roken.h.in > make-roken.c > | --- > | but it requires all macros like HAVE_FOO referred in roken.h.in > | and we have to add checks for them into src/tools/compat/configure.ac, > | as defined in src/include/heimdal/config.h configured for NetBSD. > | > | Is it really worth than adding a manually edited dumb roken.h for tools? > > Well the manually edited roken.h will need to have HAVE_FOO for each feature > in order to work across different platforms. What is currently broken in > the one we have? As far as resolv.h and arpa/nameser.h we need them elsewhere > too, so we have to fix them anyway. * src/crypto/dist/heimdal/lib/roken/roken.h.in has HAVE_FOO for each feature in order to work across different platforms: --- #ifdef HAVE_ARPA_NAMESER_H #include <arpa/nameser.h> #endif #ifdef HAVE_RESOLV_H #include <resolv.h> #endif : #if !defined(HAVE_ASNPRINTF) || defined(NEED_ASNPRINTF_PROTO) int ROKEN_LIB_FUNCTION asnprintf (char **, size_t, const char *, ...) __attribute__ ((format (printf, 3, 4))); #endif #if !defined(HAVE_VASNPRINTF) || defined(NEED_VASNPRINTF_PROTO) int ROKEN_LIB_FUNCTION vasnprintf (char **, size_t, const char *, va_list) __attribute__((format (printf, 3, 0))); #endif : int ROKEN_LIB_FUNCTION issuid(void); #ifndef HAVE_STRUCT_WINSIZE struct winsize { unsigned short ws_row, ws_col; unsigned short ws_xpixel, ws_ypixel; }; #endif int ROKEN_LIB_FUNCTION get_window_size(int fd, struct winsize *); : --- * But heimdal doesn't use it as is for a header file to be installed to a target system. * Makefiles in heimdal generate "make-roken.c" from roken.h.in using src/crypto/dist/heimdal/lib/roken/roken.awk. * generated make-roken.c is a dumb program to print "pre-processed" lines: --- #ifdef HAVE_CONFIG_H #include <config.h> #endif #include <stdio.h> int main(int argc, char **argv) { : #ifdef HAVE_ARPA_NAMESER_H puts("#include <arpa/nameser.h>"); #endif #ifdef HAVE_RESOLV_H puts("#include <resolv.h>"); #endif : #if !defined(HAVE_ASNPRINTF) || defined(NEED_ASNPRINTF_PROTO) puts("int ROKEN_LIB_FUNCTION"); puts(" asnprintf (char **, size_t, const char *, ...)"); puts(" __attribute__ ((format (printf, 3, 4)));"); #endif puts(""); #if !defined(HAVE_VASNPRINTF) || defined(NEED_VASNPRINTF_PROTO) puts("int ROKEN_LIB_FUNCTION"); puts(" vasnprintf (char **, size_t, const char *, va_list)"); puts(" __attribute__((format (printf, 3, 0)));"); #endif puts(""); : puts("int ROKEN_LIB_FUNCTION issuid(void);"); puts(""); #ifndef HAVE_STRUCT_WINSIZE puts("struct winsize {"); puts(" unsigned short ws_row, ws_col;"); puts(" unsigned short ws_xpixel, ws_ypixel;"); puts("};"); #endif puts(""); puts("int ROKEN_LIB_FUNCTION get_window_size(int fd, struct winsize *);"); puts(""); --- * Then generated roken.h (installed into src/include/heimdal/ in NetBSD) looks like: --- #include <arpa/inet.h> #include <netdb.h> #include <arpa/nameser.h> #include <resolv.h> #include <syslog.h> : int ROKEN_LIB_FUNCTION asnprintf (char **, size_t, const char *, ...) __attribute__ ((format (printf, 3, 4))); int ROKEN_LIB_FUNCTION vasnprintf (char **, size_t, const char *, va_list) __attribute__((format (printf, 3, 0))); : int ROKEN_LIB_FUNCTION issuid(void); int ROKEN_LIB_FUNCTION get_window_size(int fd, struct winsize *); --- quoted again: > Well the manually edited roken.h will need to have HAVE_FOO for each feature > in order to work across different platforms. What is currently broken in > the one we have? * As mentioned above, src/include/heimdal/roken.h doesn't have HAVE_FOO for each features in order to work across different platforms, so we should not use it for tools build. That's the broken point. * roken.h required to build src/tools/asn1_compile and src/tools/compile_et on tools hosts needs to have HAVE_FOO for each feature, but only limited definitions in roken.h.in are required by these two programs. * In my patch, I copied src/crypto/dist/heimdal/lib/roken/roken.h.in into src/tools/asn1_compile/roken.h, manually edited it, and added -I${.CURDIR} to HOST_CFLAGS in Makefiles. * I left all HAVE_FOO_H header checks and added necessary checks into src/tools/compat/configure.ac because each header might be required by <roken-common.h> included from roken.h itself. * I removed all HAVE_FEATURES checks except HAVE_STRUCT_WINSIZE from manually edited roken.h for tools because asn1_compile and compile_et builds didn't require them. So I also added a check for struct winsize in src/tools/compat/configure.ac. * I also removed all function decls except get_windows_size() because tools programs didn't use them. * I didn't change src/include/heimdal/roken.h for the next import of heimdal in future. I think this is an reasonable compromise. Do you have any better suggestion? --- Izumi Tsutsui