From 4ef59e97312cf3e8f537643c65bc2fef057b786b Mon Sep 17 00:00:00 2001 From: Nick Bowler Date: Fri, 5 Jan 2024 00:17:24 -0500 Subject: [PATCH] Avoid gnulib getline module. Outside of the test suite, we only use getline in one location. Instead of pulling in gnulib's replacement implementation and its dependencies, adjust the do_getline funciton to directly incorporate a pure standard C fallback. Since there is also some getline use in the test suite, rearrange the code a bit so that we can also include do_getline in the tests. The gnulib configure tests for getline seem to be complete overkill. According to the gnulib documentation, the portability problems which gnulib solves are: (a) The function is missing on some platforms. (b) On FreeBSD 8, getline crashes if passed a null pointer and a nonzero size. (c) On AIX 7.1, there is no declaration in . (d) Some unspecified system has an unrelated getline function which may erroneously satisfy link tests. It is unclear to me that gnulib actually solves (c) in a reasonable way. On AIX 7.2, getline is properly declared in but gnulib rejects it anyway and substitutes its own reimplementation as if it didn't exist at all. Presumably the exact same thing happens on AIX 7.1. So I don't see any reason for the annoying runtime tests just to check if we have issue (d) and linked an irrelevant getline. Although it's hard to know for sure since I don't know what system has this problem, surely it is good enough to just check that getline is properly declared in and that we can successfully link against it. This check can be implemented with just one single AC_LINK_IFELSE invocation. I feel that issue (b) is much more sensible to work around in the source code (by initializing the size to zero), that way we don't need to probe for it at all or reject this implementation. Funnily enough there is an unmentioned bug in the AIX 7.2 getline which is not the reason it gets rejected by gnulib: passing a buffer size of exactly one byte causes it to return 0 immediately instead of actually reading input and enlarging the buffer. Work around that problem too in the rendertest application (the only place that can hit it). --- Makefile.am | 2 +- configure.ac | 13 ++++++- m4/gnulib-cache.m4 | 2 - src/cdecl99.c | 20 ++-------- src/getline.h | 95 ++++++++++++++++++++++++++++++++++++++++++++++ src/version.h | 4 +- t/crossparse.c | 25 ++++-------- t/normalize.c | 14 +++---- t/rendertest.c | 13 ++++--- t/test.h | 1 + t/testlib.c | 1 + tests/general.at | 9 +++++ 12 files changed, 143 insertions(+), 56 deletions(-) create mode 100644 src/getline.h diff --git a/Makefile.am b/Makefile.am index 7b60f4b..c32eb92 100644 --- a/Makefile.am +++ b/Makefile.am @@ -73,7 +73,7 @@ EXTRA_libgnu_a_DEPENDENCIES = $(static_gl_objects) dummy $(static_gl_objects): $(gnulib_headers) bin_PROGRAMS = cdecl99 -cdecl99_SOURCES = common/src/help.c src/commands.c src/cdecl99.h +cdecl99_SOURCES = common/src/help.c src/commands.c src/cdecl99.h src/getline.h EXTRA_cdecl99_DEPENDENCIES = $(libmain_a_OBJECTS) $(libexec_a_OBJECTS) cdecl99_LDADD = $(EXTRA_cdecl99_DEPENDENCIES) libcdecl.la libgnu.a \ $(LTLIBINTL) $(LTLIBREADLINE) diff --git a/configure.ac b/configure.ac index 645e871..435eaf9 100644 --- a/configure.ac +++ b/configure.ac @@ -1,4 +1,4 @@ -dnl Copyright © 2011-2013, 2020-2023 Nick Bowler +dnl Copyright © 2011-2013, 2020-2024 Nick Bowler dnl dnl License WTFPL2: Do What The Fuck You Want To Public License, version 2. dnl This is free software: you are free to do what the fuck you want to. @@ -110,5 +110,16 @@ AH_BOTTOM([#include ]) AC_CONFIG_TESTDIR([.], [t:.]) DX_PROG_AUTOTEST_AM +AC_CACHE_CHECK([for getline], [dx_cv_have_getline], + [AC_LINK_IFELSE([AC_LANG_PROGRAM([#include +], [ssize_t (*x)() = getline; +char *p = 0; +size_t n = 0; +return getline(&p, &n, stdin); +])], [dx_cv_have_getline=yes], [dx_cv_have_getline=no])]) +AS_CASE([$dx_cv_have_getline], [yes], + [AC_DEFINE([HAVE_GETLINE], [1], + [Define to 1 if the getline function is available.])]) + AC_CONFIG_FILES([Makefile]) AC_OUTPUT diff --git a/m4/gnulib-cache.m4 b/m4/gnulib-cache.m4 index 870a647..13e0fa2 100644 --- a/m4/gnulib-cache.m4 +++ b/m4/gnulib-cache.m4 @@ -42,7 +42,6 @@ # --avoid=gperf \ # --avoid=std-gnu11 \ # dx-nls \ -# getline \ # getopt-gnu \ # gitlog-to-changelog \ # inttypes-incomplete \ @@ -55,7 +54,6 @@ gl_LOCAL_DIR([lib/local]) gl_MODULES([ dx-nls - getline getopt-gnu gitlog-to-changelog inttypes-incomplete diff --git a/src/cdecl99.c b/src/cdecl99.c index 574d36e..3eb7cc1 100644 --- a/src/cdecl99.c +++ b/src/cdecl99.c @@ -1,6 +1,6 @@ /* * Command line utility for making sense of C declarations. - * Copyright © 2011-2012, 2020-2023 Nick Bowler + * Copyright © 2011-2012, 2020-2024 Nick Bowler * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -37,6 +37,7 @@ #include "xtra.h" #include "options.h" #include "version.h" +#include "getline.h" #if HAVE_READLINE_READLINE_H # include @@ -115,21 +116,6 @@ static void print_help(const struct option *lopts) printf(_("Report bugs to <%s>.\n"), PACKAGE_BUGREPORT); } -static int do_getline(char **linebuf, size_t *n) -{ - ssize_t rc; - - if ((rc = getline(linebuf, n, stdin)) < 0) { - if (ferror(stdin)) - print_error("%s", strerror(errno)); - return 0; - } - - if (rc-- && (*linebuf)[rc] == '\n') - (*linebuf)[rc] = '\0'; - return 1; -} - static int do_readline(char **linebuf, size_t *n, bool batch) { #if !HAVE_READLINE @@ -156,7 +142,7 @@ static int repl(bool batch) { char *line = NULL; bool fail = 0; - size_t n; + size_t n = 0; while (do_readline(&line, &n, batch)) { int rc = run_command(line, batch); diff --git a/src/getline.h b/src/getline.h new file mode 100644 index 0000000..f4116a0 --- /dev/null +++ b/src/getline.h @@ -0,0 +1,95 @@ +/* + * Copyright © 2024 Nick Bowler + * + * getline-like function which removes trailing newline (if any), and + * returns nonzero if and only if a line was successfully read. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#include +#include +#include +#include + +#ifdef CDECL_TEST_H_ +/* + * Use the minimum initial alloc size in the test programs to ensure we at + * least are exercising the realloc path on a reasonably regular basis. + */ +# define CDECL99_GETLINE_INITIAL_ALLOC 1 +#else +# define CDECL99_GETLINE_INITIAL_ALLOC 75 +#endif + +static inline int do_getline(char **linebuf, size_t *n) +{ + extern void print_error(const char *fmt, ...); + const char *errmsg; + +#if HAVE_GETLINE + ssize_t rc; + + if ((rc = getline(linebuf, n, stdin)) < 0) { + if (ferror(stdin)) + goto input_error; + return 0; + } + + if (rc-- && (*linebuf)[rc] == '\n') + (*linebuf)[rc] = '\0'; + return 1; +#else + char *work = *linebuf; + size_t pos = 0; + size_t sz; + + if (!work) { + sz = CDECL99_GETLINE_INITIAL_ALLOC; + goto initial_alloc; + } + + for (sz = *n;;) { + if (!fgets(&work[pos], sz - pos, stdin)) { + if (ferror(stdin)) + goto input_error; + + return !!pos; + } + + pos += strlen(&work[pos]); + if (work[pos-1] == '\n') { + work[pos-1] = '\0'; + return 1; + } + + if (sz > INT_MAX/2 || sz > ((size_t)-1)/4) + break; + + sz = ((sz*4) + 2) / 3; +initial_alloc: + work = realloc(work, sz); + if (!work) + break; + *linebuf = work; + *n = sz; + } + + errmsg = _("failed to allocate memory"); +#endif + if (0) input_error: errmsg = strerror(errno); + print_error("%s", errmsg); + return 0; +} + diff --git a/src/version.h b/src/version.h index 9d41d11..347a416 100644 --- a/src/version.h +++ b/src/version.h @@ -1,5 +1,5 @@ /* - * Copyright © 2023 Nick Bowler + * Copyright © 2023-2024 Nick Bowler * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -20,7 +20,7 @@ #define version_blurb(s, copyright) \ s "\n" \ - copyright "2023 Nick Bowler\n" \ + copyright "2024 Nick Bowler\n" \ "License GPLv3+: GNU GPL version 3 or any later version.\n" \ "This is free software: you are free to change and redistribute it.\n" \ "There is NO WARRANTY, to the extent permitted by law." \ diff --git a/t/crossparse.c b/t/crossparse.c index e77a1cc..5f7eddc 100644 --- a/t/crossparse.c +++ b/t/crossparse.c @@ -28,6 +28,7 @@ #define PROGNAME "crossparse" #include "test.h" +#include "getline.h" static const char sopts[] = "f:ECVH"; static const struct option lopts[] = { @@ -139,7 +140,6 @@ int main(int argc, char **argv) int i; const char *filename = NULL; - FILE *infile = NULL; if (argc > 0) progname = argv[0]; @@ -153,7 +153,6 @@ int main(int argc, char **argv) mode = MODE_ENGLISH; break; case 'f': - infile = stdin; filename = optarg; break; case 'V': @@ -168,29 +167,19 @@ int main(int argc, char **argv) } } - if (infile) { + if (filename && !freopen(filename, "r", stdin)) { + print_error("%s: %s", filename, strerror(errno)); + return EXIT_FAILURE; + } else if (filename) { char *line = NULL; - size_t n; - - if (filename) { - infile = fopen(filename, "r"); - if (!infile) { - print_error("%s: %s", filename, - strerror(errno)); - return EXIT_FAILURE; - } - } + size_t n = 0; - while (getline(&line, &n, infile) >= 0) { - char *c = strchr(line, '\n'); - if (c) - *c = '\0'; + while (do_getline(&line, &n)) { if (!test_crossparse(line, mode)) ret = EXIT_FAILURE; } free(line); - fclose(infile); } else if (argv[optind]) { for (i = optind; i < argc; i++) { if (!test_crossparse(argv[i], mode)) diff --git a/t/normalize.c b/t/normalize.c index e11e648..e309d70 100644 --- a/t/normalize.c +++ b/t/normalize.c @@ -27,6 +27,7 @@ #define PROGNAME "normalize" #include "test.h" +#include "getline.h" static const char sopts[] = "f:VH"; static const struct option lopts[] = { @@ -191,17 +192,12 @@ int main(int argc, char **argv) } } - if (filename) { - infile = fopen(filename, "r"); - if (!infile) { - perror(filename); - return EXIT_FAILURE; - } + if (filename && !freopen(filename, "r", stdin)) { + print_error("%s: %s", filename, strerror(errno)); + return EXIT_FAILURE; } - while ((rc = getline(&line, &n, infile)) >= 0) { - if (rc > 0 && line[rc-1] == '\n') - line[rc-1] = 0; + while (do_getline(&line, &n)) { if (do_normalize(line, n) < 0) ret = EXIT_FAILURE; } diff --git a/t/rendertest.c b/t/rendertest.c index cd14229..00e0c59 100644 --- a/t/rendertest.c +++ b/t/rendertest.c @@ -27,6 +27,7 @@ #define PROGNAME "rendertest" #include "test.h" +#include "getline.h" static const char sopts[] = "n:ECVH"; static const struct option lopts[] = { @@ -125,12 +126,12 @@ int main(int argc, char **argv) } } - line = malloc_nofail((sz = n+1)); - while (getline(&line, &sz, stdin) >= 0) { - char *c = strchr(line, '\n'); - if (c) - *c = '\0'; - + /* + * Ensure the preallocated buffer is more than one byte, otherwise we + * will hit a bug in AIX 7.2 getline and fall into an infinite loop. + */ + line = malloc_nofail((sz = MAX(n+1, 10))); + while (do_getline(&line, &sz)) { if (do_test(line, n, mode) < 0) ret = EXIT_FAILURE; } diff --git a/t/test.h b/t/test.h index 9e8ddee..2e7882d 100644 --- a/t/test.h +++ b/t/test.h @@ -28,6 +28,7 @@ #endif #define MIN(a, b) ((a) < (b) ? (a) : (b)) +#define MAX(a, b) ((a) > (b) ? (a) : (b)) struct cdecl_declspec; struct option; diff --git a/t/testlib.c b/t/testlib.c index 10b27ad..dd9e073 100644 --- a/t/testlib.c +++ b/t/testlib.c @@ -20,6 +20,7 @@ #include #include #include +#include #include #include diff --git a/tests/general.at b/tests/general.at index f73f781..c40d6b5 100644 --- a/tests/general.at +++ b/tests/general.at @@ -230,3 +230,12 @@ AT_CHECK([echo >>stdout; sed -f check.sed stdout], [0], ]) AT_CLEANUP + +AT_SETUP([cdecl99 EOF in batch mode]) + +AT_CHECK([AS_ECHO_N(["explain int"]) | cdecl99 --batch | head], [0], +[[type int +]]) +AT_CHECK([cdecl99 --batch