Matthias Kilian <[email protected]> writes:
> New diff, where I only changed the debug stuff (and introduced a
> new typedef for it).
Your change is OK by me.
> The reason I didn't touch the other message
> printing functions is that they call printf(3)-like functions several
> times and don't do any error checking at all, and it's not clear
> what to do with those functions.
One step at a time sounds good to me.
> Fortunately, changing vdebugBelch() / debugMsgFn, rtsDebugMsgFn()
> are enough to kill the %n, and rtsDebugMsgFn() calls vfprintf(3) only
> once, so it's easy to let rtsDebugMsgFn() return the result of
> vfprintf(3).
>
> If upstream is interested, the next thing would be to remove those
> (unnecessary) indirections via function pointers.
Sure, might as well discuss this with them while landing your patch.
Thanks
Greg
>
> Ciao,
> Kili
>
> Index: Makefile
> ===================================================================
> RCS file: /cvs/ports/lang/ghc/Makefile,v
> retrieving revision 1.188
> diff -u -p -r1.188 Makefile
> --- Makefile 16 Aug 2021 21:23:18 -0000 1.188
> +++ Makefile 12 Sep 2021 19:40:01 -0000
> @@ -19,6 +19,8 @@ DISTNAME = ghc-${GHC_VERSION}
> CATEGORIES = lang devel
> HOMEPAGE = https://www.haskell.org/ghc/
>
> +REVISION = 0
> +
> # Version of the precompiled binaries
> BIN_VER = 8.10.3.20210429
>
> Index: patches/patch-includes_rts_Messages_h
> ===================================================================
> RCS file: patches/patch-includes_rts_Messages_h
> diff -N patches/patch-includes_rts_Messages_h
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ patches/patch-includes_rts_Messages_h 12 Sep 2021 19:40:01 -0000
> @@ -0,0 +1,34 @@
> +$OpenBSD$
> +
> +The debug message function has to return the number of bytes written
> +(like printf(3)), to allow killing a %n format specifier in one in
> +one invocation of statsPrintf() in report_summary() (rts/Stats.c).
> +
> +Index: includes/rts/Messages.h
> +--- includes/rts/Messages.h.orig
> ++++ includes/rts/Messages.h
> +@@ -85,20 +85,21 @@ void vsysErrorBelch(const char *s, va_list ap);
> + void debugBelch(const char *s, ...)
> + GNUC3_ATTRIBUTE(format (PRINTF, 1, 2));
> +
> +-void vdebugBelch(const char *s, va_list ap);
> ++int vdebugBelch(const char *s, va_list ap);
> +
> +
> + /* Hooks for redirecting message generation: */
> +
> + typedef void RtsMsgFunction(const char *, va_list);
> ++typedef int RtsMsgFunctionRetLen(const char *, va_list);
> +
> + extern RtsMsgFunction *fatalInternalErrorFn;
> +-extern RtsMsgFunction *debugMsgFn;
> ++extern RtsMsgFunctionRetLen *debugMsgFn;
> + extern RtsMsgFunction *errorMsgFn;
> +
> + /* Default stdio implementation of the message hooks: */
> +
> + extern RtsMsgFunction rtsFatalInternalErrorFn;
> +-extern RtsMsgFunction rtsDebugMsgFn;
> ++extern RtsMsgFunctionRetLen rtsDebugMsgFn;
> + extern RtsMsgFunction rtsErrorMsgFn;
> + extern RtsMsgFunction rtsSysErrorMsgFn;
> Index: patches/patch-rts_RtsMessages_c
> ===================================================================
> RCS file: patches/patch-rts_RtsMessages_c
> diff -N patches/patch-rts_RtsMessages_c
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ patches/patch-rts_RtsMessages_c 12 Sep 2021 19:40:01 -0000
> @@ -0,0 +1,65 @@
> +$OpenBSD$
> +
> +The debug message function has to return the number of bytes written
> +(like printf(3)), to allow killing a %n format specifier in one in
> +one invocation of statsPrintf() in report_summary() (rts/Stats.c).
> +
> +Index: rts/RtsMessages.c
> +--- rts/RtsMessages.c.orig
> ++++ rts/RtsMessages.c
> +@@ -36,7 +36,7 @@
> +
> + // Default to the stdio implementation of these hooks.
> + RtsMsgFunction *fatalInternalErrorFn = rtsFatalInternalErrorFn;
> +-RtsMsgFunction *debugMsgFn = rtsDebugMsgFn;
> ++RtsMsgFunctionRetLen *debugMsgFn = rtsDebugMsgFn;
> + RtsMsgFunction *errorMsgFn = rtsErrorMsgFn;
> + RtsMsgFunction *sysErrorMsgFn = rtsSysErrorMsgFn;
> +
> +@@ -102,10 +102,10 @@ debugBelch(const char*s, ...)
> + va_end(ap);
> + }
> +
> +-void
> ++int
> + vdebugBelch(const char*s, va_list ap)
> + {
> +- (*debugMsgFn)(s,ap);
> ++ return (*debugMsgFn)(s,ap);
> + }
> +
> + /*
> -----------------------------------------------------------------------------
> +@@ -285,16 +285,16 @@ rtsSysErrorMsgFn(const char *s, va_list ap)
> + #endif
> + }
> +
> +-void
> ++int
> + rtsDebugMsgFn(const char *s, va_list ap)
> + {
> ++ int r;
> + #if defined(mingw32_HOST_OS)
> + /* Ensure we're in text mode so newlines get encoded properly. */
> + int mode = _setmode (_fileno(stderr), _O_TEXT);
> + if (isGUIApp())
> + {
> + char buf[BUFSIZE];
> +- int r;
> +
> + r = vsnprintf(buf, BUFSIZE, s, ap);
> + if (r > 0 && r < BUFSIZE) {
> +@@ -305,12 +305,13 @@ rtsDebugMsgFn(const char *s, va_list ap)
> + #endif
> + {
> + /* don't fflush(stdout); WORKAROUND bug in Linux glibc */
> +- vfprintf(stderr, s, ap);
> ++ r = vfprintf(stderr, s, ap);
> + fflush(stderr);
> + }
> + #if defined(mingw32_HOST_OS)
> + _setmode (_fileno(stderr), mode);
> + #endif
> ++ return r;
> + }
> +
> +
> Index: patches/patch-rts_Stats_c
> ===================================================================
> RCS file: patches/patch-rts_Stats_c
> diff -N patches/patch-rts_Stats_c
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ patches/patch-rts_Stats_c 12 Sep 2021 19:40:01 -0000
> @@ -0,0 +1,54 @@
> +$OpenBSD$
> +
> +Kill a use of %n format specifier.
> +
> +Index: rts/Stats.c
> +--- rts/Stats.c.orig
> ++++ rts/Stats.c
> +@@ -69,7 +69,7 @@ static Time *GC_coll_cpu = NULL;
> + static Time *GC_coll_elapsed = NULL;
> + static Time *GC_coll_max_pause = NULL;
> +
> +-static void statsPrintf( char *s, ... ) GNUC3_ATTRIBUTE(format (PRINTF, 1,
> 2));
> ++static int statsPrintf( char *s, ... ) GNUC3_ATTRIBUTE(format (PRINTF, 1,
> 2));
> + static void statsFlush( void );
> + static void statsClose( void );
> +
> +@@ -1024,8 +1024,10 @@ static void report_summary(const RTSSummaryStats* sum)
> +
> + for (g = 0; g < RtsFlags.GcFlags.generations; g++) {
> + int prefix_length = 0;
> +- statsPrintf("%*s" "gen[%" FMT_Word32 "%n",
> +- col_width[0], "", g, &prefix_length);
> ++ prefix_length = statsPrintf("%*s" "gen[%" FMT_Word32,
> ++ col_width[0], "", g);
> ++ if (prefix_length < 0)
> ++ prefix_length = 0;
> + prefix_length -= col_width[0];
> + int suffix_length = col_width[1] + prefix_length;
> + suffix_length =
> +@@ -1735,19 +1737,21 @@ void getRTSStats( RTSStats *s )
> + Dumping stuff in the stats file, or via the debug message interface
> +
> -------------------------------------------------------------------------- */
> +
> +-void
> ++int
> + statsPrintf( char *s, ... )
> + {
> ++ int ret = 0;
> + FILE *sf = RtsFlags.GcFlags.statsFile;
> + va_list ap;
> +
> + va_start(ap,s);
> + if (sf == NULL) {
> +- vdebugBelch(s,ap);
> ++ ret = vdebugBelch(s,ap);
> + } else {
> +- vfprintf(sf, s, ap);
> ++ ret = vfprintf(sf, s, ap);
> + }
> + va_end(ap);
> ++ return ret;
> + }
> +
> + static void
