From 2a23247420094c8f576dd6ec30978fe418355336 Mon Sep 17 00:00:00 2001 From: John Bowler Date: Sun, 14 Jan 2024 15:47:20 -0800 Subject: [PATCH] pngcp: remove GNU setjmp warning workround Prior versons of the GCC warned about the 'dest' parameter of contrib/tools/pngcp.c not being volatile, which isn't necessary because it isn't modified. This removes the GCC specific fixup. The function which calls setjmp, cppng() also relied on undefined behavior because it assigned the result of setjmp() to a variable; this is not one of the four uses of setjmp permitted by ANSI-C. This passes the result previously returned by longjmp via (struct display). It's very very unlikely that any compiler could have got the code wrong but it is technically undefined. --- contrib/tools/pngcp.c | 32 +++++++++++--------------------- 1 file changed, 11 insertions(+), 21 deletions(-) diff --git a/contrib/tools/pngcp.c b/contrib/tools/pngcp.c index 579e28d2b..cf621fa5b 100644 --- a/contrib/tools/pngcp.c +++ b/contrib/tools/pngcp.c @@ -1,6 +1,6 @@ /* pngcp.c * - * Copyright (c) 2016,2022 John Cunningham Bowler + * Copyright (c) 2016,2022,2024 John Cunningham Bowler * * This code is released under the libpng license. * For conditions of distribution and use, see the disclaimer @@ -88,16 +88,6 @@ # define voidcast(type, value) (value) #endif /* __cplusplus */ -#ifdef __GNUC__ - /* Many versions of GCC erroneously report that local variables unmodified - * within the scope of a setjmp may be clobbered. This hacks round the - * problem (sometimes) without harming other compilers. - */ -# define gv volatile -#else -# define gv -#endif - /* 'CLOCK_PROCESS_CPUTIME_ID' is one of the clock timers for clock_gettime. It * need not be supported even when clock_gettime is available. It returns the * 'CPU' time the process has consumed. 'CPU' time is assumed to include time @@ -395,6 +385,7 @@ struct display { jmp_buf error_return; /* Where to go to on error */ unsigned int errset; /* error_return is set */ + int errlevel; /* error level from longjmp */ const char *operation; /* What is happening */ const char *filename; /* The name of the original file */ @@ -634,7 +625,10 @@ display_log(struct display *dp, error_level level, const char *fmt, ...) if (level > APP_FAIL || (level > ERRORS && !(dp->options & CONTINUE))) { if (dp->errset) + { + dp->errlevel = level; longjmp(dp->error_return, level); + } else exit(99); @@ -2318,14 +2312,9 @@ cp_one_file(struct display *dp, const char *filename, const char *destname) } static int -cppng(struct display *dp, const char *file, const char *gv dest) - /* Exists solely to isolate the setjmp clobbers which some versions of GCC - * erroneously generate. - */ +cppng(struct display *dp, const char *file, const char *dest) { - int ret = setjmp(dp->error_return); - - if (ret == 0) + if (setjmp(dp->error_return) == 0) { dp->errset = 1; cp_one_file(dp, file, dest); @@ -2337,10 +2326,11 @@ cppng(struct display *dp, const char *file, const char *gv dest) { dp->errset = 0; - if (ret < ERRORS) /* shouldn't longjmp on warnings */ - display_log(dp, INTERNAL_ERROR, "unexpected return code %d", ret); + if (dp->errlevel < ERRORS) /* shouldn't longjmp on warnings */ + display_log(dp, INTERNAL_ERROR, "unexpected return code %d", + dp->errlevel); - return ret; + return dp->errlevel; } }