From ad973d63e038f293f2b158f19b04c1582e616af0 Mon Sep 17 00:00:00 2001 From: Nick Bowler Date: Sat, 13 Mar 2021 17:23:46 -0500 Subject: [PATCH] Rework library error reporting. This removes (hopefully) all cases where the libcdecl prints error messages directly to stderr, and reports these messages via cdecl_get_error instead. --- Makefile.am | 7 ++- NEWS | 3 +- doc/libcdecl.3 | 27 +++++----- src/cdecl-internal.h | 2 +- src/cdecl.h | 12 ++++- src/error.c | 119 +++++++++++++++++++++---------------------- src/errtab.str | 2 - src/parse-decl.c | 42 +++++++-------- src/parse.y | 33 ++++++------ src/scan.l | 45 ++++++++++++---- 10 files changed, 161 insertions(+), 131 deletions(-) delete mode 100644 src/errtab.str diff --git a/Makefile.am b/Makefile.am index 9bef1c9..191a697 100644 --- a/Makefile.am +++ b/Makefile.am @@ -39,10 +39,10 @@ noinst_DATA = $(MOFILES) lib_LTLIBRARIES = libcdecl.la libcdecl_la_LDFLAGS = -export-symbols-regex '^cdecl_[[:lower:]]' \ - -no-undefined + -no-undefined -version-info 1:0:0 libcdecl_la_SOURCES = src/scan.c src/parse.c src/parse-decl.c src/output.c \ src/explain.c src/declare.c src/i18n.c src/error.c \ - src/normalize.c src/cdecl-internal.h src/errtab.h + src/normalize.c src/cdecl-internal.h libcdecl_la_LIBADD = libgnu.la $(LTLIBINTL) $(LTLIBTHREAD) $(libcdecl_la_OBJECTS): $(gnulib_headers) @@ -85,7 +85,6 @@ $(test_normalize_OBJECTS): $(gnulib_headers) src/parse.lo: src/scan.h src/scan.lo: src/parse.h src/parse-decl.lo: src/scan.h src/parse.h src/typemap.h -src/error.lo: src/errtab.h src/output.lo: src/specstr.h test/declgen.lo: test/typegen.h @@ -180,7 +179,7 @@ $(OPTFILES:.opt=.h): $(DX_BASEDIR)/scripts/gen-options.awk MAINTAINERCLEANFILES += $(OPTFILES:.opt=.h) EXTRA_DIST += $(DX_BASEDIR)/scripts/gen-options.awk $(OPTFILES) -STRFILES = src/commands.str src/errtab.str +STRFILES = src/commands.str .str.h: $(AM_V_GEN) $(AWK) -f $(DX_BASEDIR)/scripts/gen-strtab.awk $< >$@.tmp $(AM_V_at) mv -f $@.tmp $@ diff --git a/NEWS b/NEWS index 94edf77..f972d6e 100644 --- a/NEWS +++ b/NEWS @@ -1,5 +1,6 @@ Release 1a: - * Various bug fixes. + * Error messages are no longer printed by libcdecl. + * Various bug fixes and improvements. Release 1: * Initial release. diff --git a/doc/libcdecl.3 b/doc/libcdecl.3 index 495a353..dd70f49 100644 --- a/doc/libcdecl.3 +++ b/doc/libcdecl.3 @@ -1,4 +1,4 @@ -.Dd March 6, 2021 +.Dd March 13, 2021 .Os cdecl99 .Dt LIBCDECL \&3 "Cdecl99 Developer's Manual" .Sh NAME @@ -322,20 +322,21 @@ This error information can be retrieved by calling the function .Pp which returns a pointer to the error structure most recently generated in the current thread. -It is therefore thread-safe in that errors occurring in another thread will not -interfere with the current one. -The returned pointer shall remain valid until the next call to any function +It is therefore thread-safe in the sense that errors occurring concurrently +in another thread will not interfere with a call to +.Fn cdecl_get_error . +The returned structure shall remain valid until the next call to any function from -.Nm -by the same thread, except that multiple consecutive calls to -.Va cdecl_get_error -shall all return the same value. -The same applies to the -.Va str -pointer inside the error structure itself. +.Nm , +by the same thread, other than another call to +.Fn cdecl_get_error . .Pp -If this function is called before an error has been indicated by an earlier -call in the same thread, the behaviour is undefined. +If no prior +.Nm +call has indicated that an error occurred in the current thread, the result +from calling +.Fn cdecl_get_error +is unspecified. .Sh PARSING DECLARATIONS To parse a declaration, the function .Pp diff --git a/src/cdecl-internal.h b/src/cdecl-internal.h index 2140006..11903bc 100644 --- a/src/cdecl-internal.h +++ b/src/cdecl-internal.h @@ -30,7 +30,7 @@ struct cdecl_declspec; void cdecl__init_i18n(void); const char *cdecl__strerror(unsigned code); -void cdecl__set_error(const struct cdecl_error *err); +void cdecl__err(unsigned code, ...); unsigned long cdecl__build_typemap(struct cdecl_declspec *s); struct cdecl_declspec *cdecl__normalize_specs(struct cdecl_declspec *specs); diff --git a/src/cdecl.h b/src/cdecl.h index 11e0099..0846cfd 100644 --- a/src/cdecl.h +++ b/src/cdecl.h @@ -117,8 +117,18 @@ static inline _Bool cdecl_is_abstract(const struct cdecl_declarator *d) /* Error handling. */ enum { - CDECL_ENOMEM, + CDECL_ENOMEM = 1, CDECL_ENOPARSE, + CDECL_EBADARRAY, + CDECL_EBADDECL, + CDECL_EBADPARAMS, + CDECL_EBADPOINTER, + CDECL_EBADQUAL, + CDECL_EBADRETURN, + CDECL_EBADSTOR, + CDECL_EBADTYPE, + CDECL_ENOTFUNC, + CDECL_EVOIDPARAM }; struct cdecl_error { diff --git a/src/error.c b/src/error.c index 093f56c..9a91e4e 100644 --- a/src/error.c +++ b/src/error.c @@ -17,9 +17,11 @@ */ #include +#include #include #include #include +#include #include #include @@ -44,18 +46,6 @@ static struct err_state err_no_mem = { }, }; -const char *cdecl__strerror(unsigned code) -{ -#include "errtab.h" - - switch (code) { - case CDECL_ENOMEM: return gettext(strtab+err_enomem); - case CDECL_ENOPARSE: return gettext(strtab+err_enoparse); - } - - assert(0); -} - static void free_err(void *err) { if (err == &err_no_mem) @@ -64,59 +54,75 @@ static void free_err(void *err) free(err); } -static void set_error(const struct cdecl_error *err) +static void initialize(void) +{ + cdecl__init_i18n(); + err_no_mem.err.str = _("failed to allocate memory"); + + gl_tls_key_init(tls_key, free_err); +} + +/* + * cdecl__err(CDECL_ENOMEM); + * cdecl__err(code, fmt, ...); + * + * Sets the library error to code, with a printf-style error string. + */ +void cdecl__err(unsigned code, ...) { + const char *fmt; struct err_state *state; - size_t need_len = 0; + int rc, try = 0; + va_list ap; - if (err->str) { - need_len = strlen(err->str) + 1; - } + gl_once(tls_initialized, initialize); state = gl_tls_get(tls_key); - if (state == &err_no_mem) - state = NULL; - if (!state || state->nstr < need_len) { - struct err_state *tmp; - - tmp = realloc(state, sizeof *state + need_len); + if (!state || state == &err_no_mem) { + void *tmp = malloc(sizeof *state + 100); + if (!tmp) { - /* Re-use the existing state buffer, if any. */ - if (state) - state->err = err_no_mem.err; - else - state = &err_no_mem; - - gl_tls_set(tls_key, state); + state = &err_no_mem; return; } - state = tmp; - state->nstr = need_len; - gl_tls_set(tls_key, state); + gl_tls_set(tls_key, (state = tmp)); + state->nstr = 100; } - state->err = *err; - if (err->str) { - memcpy(state->str, err->str, need_len); - state->err.str = state->str; - } else { - state->err.str = cdecl__strerror(state->err.code); + if (code == CDECL_ENOMEM) { + if (state != &err_no_mem) + state->err = err_no_mem.err; + return; } -} - -static void initialize(void) -{ - cdecl__init_i18n(); - err_no_mem.err.str = cdecl__strerror(CDECL_ENOMEM); +retry: + va_start(ap, code); + fmt = va_arg(ap, const char *); + rc = vsnprintf(state->str, state->nstr, fmt, ap); + va_end(ap); + + if (rc > 0 && rc >= state->nstr) { + void *tmp; + size_t n; + + assert(try == 0 && rc < SIZE_MAX / 4); + + n = ((size_t)rc + 1) * 2; + tmp = realloc(state, sizeof *state + n); + if (tmp) { + state = tmp; + state->nstr = n; + try++; + + goto retry; + } - gl_tls_key_init(tls_key, free_err); + state->err = err_no_mem.err; + return; + } - /* - * This default error message is a stop-gap measure until all library - * error conditions use the new interface. - */ - set_error(&(const struct cdecl_error){ .code = CDECL_ENOPARSE }); + state->err.str = state->str; + state->err.code = code; } const struct cdecl_error *cdecl_get_error(void) @@ -126,14 +132,5 @@ const struct cdecl_error *cdecl_get_error(void) gl_once(tls_initialized, initialize); state = gl_tls_get(tls_key); - assert(state); - - return &state->err; -} - -void cdecl__set_error(const struct cdecl_error *err) -{ - gl_once(tls_initialized, initialize); - - set_error(err); + return state ? &state->err : NULL; } diff --git a/src/errtab.str b/src/errtab.str deleted file mode 100644 index 284f71a..0000000 --- a/src/errtab.str +++ /dev/null @@ -1,2 +0,0 @@ -&err_enomem failed to allocate memory -&err_enoparse syntax error diff --git a/src/parse-decl.c b/src/parse-decl.c index f25c4c4..f00442d 100644 --- a/src/parse-decl.c +++ b/src/parse-decl.c @@ -63,9 +63,9 @@ static int valid_typespec(struct cdecl_declspec *s) if (map & bit) { if (bit == 1ul << MAP_LLONG_BIT) - fprintf(stderr, "too many long specifiers\n"); + cdecl__err(CDECL_EBADTYPE, _("too many long specifiers")); else - fprintf(stderr, "duplicate type specifier\n"); + cdecl__err(CDECL_EBADTYPE, _("duplicate type specifier")); return false; } map |= bit; @@ -75,9 +75,9 @@ static int valid_typespec(struct cdecl_declspec *s) return true; if (map == 0) - fprintf(stderr, "no type specified\n"); + cdecl__err(CDECL_EBADTYPE, _("no type specified")); else - fprintf(stderr, "invalid type specified\n"); + cdecl__err(CDECL_EBADTYPE, _("invalid type specified")); return false; } @@ -103,23 +103,23 @@ static bool valid_declspecs(struct cdecl *decl, bool top) if (c->type == CDECL_TYPE_VOID && (d->type == CDECL_DECL_IDENT || d->type == CDECL_DECL_ARRAY)) { - fprintf(stderr, "invalid declaration of type void\n"); + cdecl__err(CDECL_EBADTYPE, _("invalid declaration of type void")); return false; } continue; case CDECL_SPEC_STOR: if (top && abstract) { - fprintf(stderr, "type names cannot have storage-class specifiers\n"); + cdecl__err(CDECL_EBADSTOR, _("type names cannot have storage-class specifiers")); return false; } if (!top && c->type != CDECL_STOR_REGISTER) { - fprintf(stderr, "function parameters may only have register storage\n"); + cdecl__err(CDECL_EBADSTOR, _("function parameters may only have register storage")); return false; } if (++num_storage > 1) { - fprintf(stderr, "too many storage-class specifiers\n"); + cdecl__err(CDECL_EBADSTOR, _("too many storage-class specifiers")); return false; } break; @@ -129,18 +129,18 @@ static bool valid_declspecs(struct cdecl *decl, bool top) * pointer qualifier list, which isn't checked here. */ if (c->type == CDECL_QUAL_RESTRICT) { - fprintf(stderr, "only pointer types can be restrict-qualified.\n"); + cdecl__err(CDECL_EBADQUAL, _("only pointer types can be restrict-qualified")); return false; } break; case CDECL_SPEC_FUNC: if (abstract) { - fprintf(stderr, "type names cannot have function specifiers\n"); + cdecl__err(CDECL_ENOTFUNC, _("type names cannot have function specifiers")); return false; } if (!top || d->type != CDECL_DECL_FUNCTION) { - fprintf(stderr, "only function declarations may have function specifiers.\n"); + cdecl__err(CDECL_ENOTFUNC, _("only function declarations can have function specifiers")); return false; } break; @@ -280,7 +280,7 @@ reduce_parentheses(struct cdecl_declarator **p, struct cdecl_declarator *d) } if (d->child->type != CDECL_DECL_NULL) { - fprintf(stderr, "invalid function parameter\n"); + cdecl__err(CDECL_EBADPARAMS, _("invalid function parameter")); return -1; } @@ -304,7 +304,7 @@ reduce_parentheses(struct cdecl_declarator **p, struct cdecl_declarator *d) if (decl->type == CDECL_DECL_FUNCTION && decl->child->type == CDECL_DECL_NULL && !function_is_reducible(decl)) { - fprintf(stderr, "too many parentheses in function\n"); + cdecl__err(CDECL_EBADPARAMS, _("too many parentheses in function")); return -1; } @@ -339,13 +339,13 @@ check_parameters(struct cdecl_declarator **p, struct cdecl_declarator *d) continue; if (spec != param->specifiers || spec->next != NULL) { - fprintf(stderr, "void parameter must not have extra specifiers\n"); + cdecl__err(CDECL_EVOIDPARAM, _("void parameter cannot have extra specifiers")); return -1; } else if (d->u.function.parameters->next) { - fprintf(stderr, "a void parameter must stand alone\n"); + cdecl__err(CDECL_EVOIDPARAM, _("void parameter must stand alone")); return -1; } else if (d->u.function.variadic) { - fprintf(stderr, "variadic functions cannot have a void parameter\n"); + cdecl__err(CDECL_EVOIDPARAM, _("variadic function cannot have void parameter")); return -1; } } @@ -366,10 +366,10 @@ check_rettypes(struct cdecl_declarator **p, struct cdecl_declarator *d) switch (d->type) { case CDECL_DECL_FUNCTION: - fprintf(stderr, "functions cannot return functions\n"); + cdecl__err(CDECL_EBADRETURN, _("functions cannot return functions")); return -1; case CDECL_DECL_ARRAY: - fprintf(stderr, "functions cannot return arrays\n"); + cdecl__err(CDECL_EBADRETURN, _("functions cannot return arrays")); return -1; } @@ -384,7 +384,7 @@ check_arrays(struct cdecl_declarator **p, struct cdecl_declarator *d) switch (d->type) { case CDECL_DECL_FUNCTION: - fprintf(stderr, "array members cannot be functions\n"); + cdecl__err(CDECL_EBADARRAY, _("array members cannot be functions")); return -1; } @@ -425,7 +425,7 @@ check_qualifiers(struct cdecl_declarator **p, struct cdecl_declarator *d) for (spec = ptr->qualifiers; spec; spec = spec->next) { if (spec->type == CDECL_QUAL_RESTRICT && d->type == CDECL_DECL_FUNCTION) { - fprintf(stderr, "function pointers cannot be restrict-qualified\n"); + cdecl__err(CDECL_EBADPOINTER, _("function pointers cannot be restrict-qualified")); return -1; } } @@ -524,7 +524,7 @@ struct cdecl *cdecl_parse_decl(const char *declstr) if (cdecl_is_abstract(i->declarators) && (i != decl || i->next)) { - fprintf(stderr, "mixing type names and declarations is not allowed\n"); + cdecl__err(CDECL_EBADDECL, _("mixing type names and declarations is not allowed")); goto err; } } diff --git a/src/parse.y b/src/parse.y index 11ae6bb..bf2a596 100644 --- a/src/parse.y +++ b/src/parse.y @@ -42,8 +42,10 @@ #define ALLOC(ptr, size) do { \ (ptr) = malloc(size); \ - if (!(ptr)) \ - FAIL("failed to allocate memory"); \ + if (!(ptr)) { \ + cdecl__err(CDECL_ENOMEM); \ + YYERROR; \ + } \ } while (0) #define ALLOC_STRUCT(ptr, type, ...) do { \ @@ -58,7 +60,6 @@ %code provides { void cdecl__free(struct cdecl *); -void cdecl__yyerror(YYLTYPE *, void *, struct cdecl **, const char *); int cdecl__yyparse(void *scanner, struct cdecl **out); } @@ -138,6 +139,15 @@ void cdecl__free(struct cdecl *decl) { free_decl(decl); } + +static void +yyerror(YYLTYPE *loc, yyscan_t scanner, struct cdecl **out, const char *err) +{ + if (strstr(err, "T_LEX_ERROR")) + return; + + cdecl__err(CDECL_ENOPARSE, "%s", err); +} %} %destructor { free($$); } @@ -336,7 +346,7 @@ vla_ident: T_IDENT | T_ASTERISK { array: T_LBRACKET T_UINT T_RBRACKET { if ($2 == 0) - FAIL("array length must be positive"); + FAIL(_("array length must be positive")); ALLOC_STRUCT($$, struct cdecl_declarator, .type = CDECL_DECL_ARRAY, @@ -563,7 +573,7 @@ english_array: T_VLA T_ARRAY english_vla T_OF { .u.array.vla = $3); } | T_ARRAY T_UINT T_OF { if ($2 == 0) - FAIL("array length must be positive"); + FAIL(_("array length must be positive")); ALLOC_STRUCT($$, struct cdecl_declarator, .type = CDECL_DECL_ARRAY, @@ -578,16 +588,3 @@ english_vla: T_IDENT | { ALLOC($$, sizeof ""); strcpy($$, ""); } - -%% -void -yyerror(YYLTYPE *loc, yyscan_t scanner, struct cdecl **out, const char *err) -{ - if (strstr(err, "T_LEX_ERROR")) - return; - - cdecl__set_error(&(const struct cdecl_error){ - .code = CDECL_ENOPARSE, - .str = err, - }); -} diff --git a/src/scan.l b/src/scan.l index 54cea88..cc152b9 100644 --- a/src/scan.l +++ b/src/scan.l @@ -26,15 +26,21 @@ %option prefix="cdecl__yy" %{ -#define lex_error(msg) do { \ - cdecl__yyerror(yylloc, NULL, NULL, (msg)); \ +#include +#include "cdecl-internal.h" +#include "cdecl.h" + +#define lex_error(...) do { \ + cdecl__err(CDECL_ENOPARSE, __VA_ARGS__); \ return T_LEX_ERROR; \ } while(0) #define dup_token() do { \ yylval->strval = malloc(yyleng+1); \ - if (!yylval->strval) \ - lex_error("failed to allocate memory"); \ + if (!yylval->strval) { \ + cdecl__err(CDECL_ENOMEM); \ + return T_LEX_ERROR; \ + } \ strcpy(yylval->strval, yytext); \ } while(0) %} @@ -98,9 +104,9 @@ INTEGER 0x[[:xdigit:]]+|0[0-7]+|[[:digit:]]+ errno = 0; yylval->uintval = strtoumax(yytext, &end, 0); if (errno == ERANGE) - lex_error("integer constant out of range"); + lex_error(_("integer constant out of range")); if (*end) - lex_error("invalid integer constant"); + lex_error(_("invalid integer constant")); return T_UINT; } @@ -122,7 +128,28 @@ INTEGER 0x[[:xdigit:]]+|0[0-7]+|[[:digit:]]+ [[:space:]]+ . { - char buf[] = "syntax error, unexpected #"; - *strchr(buf, '#') = *yytext; - lex_error(buf); + char buf[5] = { yytext[0] }; + unsigned char c = buf[0]; + + if (!isprint(c) || c == '\\' || c == '\'') { + /* Encode nonprinting characters with C-style escapes */ + buf[0] = '\\'; + switch (c) { + case '\a': buf[1] = 'a'; break; + case '\b': buf[1] = 'b'; break; + case '\f': buf[1] = 'f'; break; + case '\n': buf[1] = 'n'; break; + case '\r': buf[1] = 'r'; break; + case '\t': buf[1] = 't'; break; + case '\v': buf[1] = 'v'; break; + case '\\': buf[1] = '\\'; break; + case '\'': buf[1] = '\''; break; + default: + buf[1] = '0' + ((c >> 6) & 3); + buf[2] = '0' + ((c >> 3) & 7); + buf[3] = '0' + ((c >> 0) & 7); + } + } + + lex_error(_("syntax error, unexpected '%s'"), buf); } -- 2.43.2