From: PatR Date: Fri, 1 Jul 2022 20:08:43 +0000 (-0700) Subject: regex handling X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=dda4cd0530207b2925ae2d46f59b498bfcb464d2;p=nethack regex handling Change the regex_error_desc() interface. Have the caller pass in a pointer to a buffer of at least BUFSZ characters and have regex_error_desc() populate that. No need for static buffers or extra dynamic alloction. Also, change it to never return Null. None of its callers were checking for that and could have passed Null to config_error_add() or raw_print(). printf("%s", NULL) produces "null" on OSX but other systems would probably crash if a Null result ever actually occurred. The error explanation returned by cppregex included a trailing period. config_error_add() adds one, so the message ended up with two. Have regex_error_desc() check for final period and strip it off if found. (My test case used a menucolor pattern of "[" which triggers an error about mismatched brackets.) Reformat cppregex.cpp; treat 'extern "C" {' as if it isn't introducing a nested block. Fix the '#include ' that 'make depend' was ignoring. --- diff --git a/include/extern.h b/include/extern.h index 9fc3bb929..711a0d621 100644 --- a/include/extern.h +++ b/include/extern.h @@ -1829,7 +1829,7 @@ extern void nhl_pushhooked_open_table(lua_State *L); extern struct nhregex *regex_init(void); extern boolean regex_compile(const char *, struct nhregex *); -extern const char *regex_error_desc(struct nhregex *); +extern char *regex_error_desc(struct nhregex *, char *); extern boolean regex_match(const char *, struct nhregex *); extern void regex_free(struct nhregex *); diff --git a/src/options.c b/src/options.c index 1acff8f3f..07fe3598b 100644 --- a/src/options.c +++ b/src/options.c @@ -6916,7 +6916,8 @@ msgtype_add(int typ, char *pattern) /* test_regex_pattern() has already validated this regexp but parsing it again could conceivably run out of memory */ if (!regex_compile(pattern, tmp->regex)) { - const char *re_error_desc = regex_error_desc(tmp->regex); + char errbuf[BUFSZ]; + char *re_error_desc = regex_error_desc(tmp->regex, errbuf); /* free first in case reason for failure was insufficient memory */ regex_free(tmp->regex); @@ -7048,7 +7049,7 @@ test_regex_pattern(const char *str, const char *errmsg) { static const char def_errmsg[] = "NHregex error"; struct nhregex *match; - const char *re_error_desc; + char *re_error_desc, errbuf[BUFSZ]; boolean retval; if (!str) @@ -7066,7 +7067,7 @@ test_regex_pattern(const char *str, const char *errmsg) /* get potential error message before freeing regexp and free regexp before issuing message in case the error is "ran out of memory" since message delivery might need to allocate some memory */ - re_error_desc = !retval ? regex_error_desc(match) : 0; + re_error_desc = !retval ? regex_error_desc(match, errbuf) : 0; /* discard regexp; caller will re-parse it after validating other stuff */ regex_free(match); /* if returning failure, tell player */ @@ -7089,7 +7090,8 @@ add_menu_coloring_parsed(const char *str, int c, int a) /* test_regex_pattern() has already validated this regexp but parsing it again could conceivably run out of memory */ if (!regex_compile(str, tmp->match)) { - const char *re_error_desc = regex_error_desc(tmp->match); + char errbuf[BUFSZ]; + char *re_error_desc = regex_error_desc(tmp->match, errbuf); /* free first in case reason for regcomp failure was out-of-memory */ regex_free(tmp->match); @@ -8242,7 +8244,8 @@ add_autopickup_exception(const char *mapping) ape = (struct autopickup_exception *) alloc(sizeof *ape); ape->regex = regex_init(); if (!regex_compile(text, ape->regex)) { - const char *re_error_desc = regex_error_desc(ape->regex); + char errbuf[BUFSZ]; + char *re_error_desc = regex_error_desc(ape->regex, errbuf); /* free first in case reason for failure was insufficient memory */ regex_free(ape->regex); diff --git a/src/sounds.c b/src/sounds.c index 9df6ad572..bb4f62746 100644 --- a/src/sounds.c +++ b/src/sounds.c @@ -1453,7 +1453,8 @@ add_sound_mapping(const char* mapping) new_map->next = soundmap; if (!regex_compile(text, new_map->regex)) { - const char *re_error_desc = regex_error_desc(new_map->regex); + char errbuf[BUFSZ]; + char *re_error_desc = regex_error_desc(new_map->regex, errbuf); regex_free(new_map->regex); free((genericptr_t) new_map->filename); diff --git a/sys/share/cppregex.cpp b/sys/share/cppregex.cpp index 0e3a7de1a..3f2cbe622 100644 --- a/sys/share/cppregex.cpp +++ b/sys/share/cppregex.cpp @@ -8,57 +8,83 @@ /* nhregex interface documented in sys/share/posixregex.c */ -extern "C" { - #include +extern "C" { // rest of file - extern const char regex_id[] = "cppregex"; - static char cppregex_static_buffer[BUFSZ]; +#include "config.h" - struct nhregex { +extern const char regex_id[] = "cppregex"; + +struct nhregex { std::unique_ptr re; std::unique_ptr err; - }; +}; - struct nhregex *regex_init(void) { +struct nhregex * +regex_init(void) +{ return new nhregex; - } +} - boolean regex_compile(const char *s, struct nhregex *re) { +boolean +regex_compile(const char *s, struct nhregex *re) +{ if (!re) - return FALSE; + return FALSE; try { - re->re.reset(new std::regex(s, (std::regex::extended - | std::regex::nosubs - | std::regex::optimize))); - re->err.reset(nullptr); - return TRUE; + re->re.reset(new std::regex(s, (std::regex::extended + | std::regex::nosubs + | std::regex::optimize))); + re->err.reset(nullptr); + return TRUE; } catch (const std::regex_error& err) { - re->err.reset(new std::regex_error(err)); - re->re.reset(nullptr); - return FALSE; + re->err.reset(new std::regex_error(err)); + re->re.reset(nullptr); + return FALSE; } - } +} + +char * +regex_error_desc(struct nhregex *re, char *errbuf) +{ + if (!re) { + Strcpy(errbuf, "no regexp"); + } else if (!re->err) { + Strcpy(errbuf, "no explanation"); + } else { + char *p; + + errbuf[0] = '\0'; + (void) strncat(errbuf, re->err->what(), BUFSZ - 1); + if (!errbuf[0]) + Strcpy(errbuf, "unspecified regexp error"); - const char *regex_error_desc(struct nhregex *re) { - if (re->err) { - Snprintf(cppregex_static_buffer, sizeof cppregex_static_buffer, - "%s", re->err->what()); - return cppregex_static_buffer; - } else - return nullptr; - } + /* caller will pass our result to config_error_add() and it adds + sentence ending period, so if the regex class error explanation + already ends in a period, strip that off */ + if ((p = strrchr(errbuf, '.')) != 0 && !p[1]) + *p = '\0'; + } + return errbuf; +} - boolean regex_match(const char *s, struct nhregex *re) { +boolean +regex_match(const char *s, struct nhregex *re) +{ if (!re->re) - return false; + return false; try { - return regex_search(s, *re->re, std::regex_constants::match_any); + return regex_search(s, *re->re, std::regex_constants::match_any); } catch (const std::regex_error& err) { - return false; + return false; } - } +} - void regex_free(struct nhregex *re) { +void +regex_free(struct nhregex *re) +{ delete re; - } } + +} // extern "C" + +/*cppregex.cpp*/ diff --git a/sys/share/pmatchregex.c b/sys/share/pmatchregex.c index d04fdacc1..0cf477a8c 100644 --- a/sys/share/pmatchregex.c +++ b/sys/share/pmatchregex.c @@ -42,9 +42,9 @@ regex_compile(const char *s, struct nhregex *re) } const char * -regex_error_desc(struct nhregex *re UNUSED) +regex_error_desc(struct nhregex *re UNUSED, char *errbuf) { - return "pattern match compilation error"; + return strcpy(errbuf, "pattern match compilation error"); } boolean @@ -65,3 +65,5 @@ regex_free(struct nhregex *re) free((genericptr_t) re); } } + +/*pmatchregex.c*/ diff --git a/sys/share/posixregex.c b/sys/share/posixregex.c index 37f16ada9..d6b3c4845 100644 --- a/sys/share/posixregex.c +++ b/sys/share/posixregex.c @@ -29,12 +29,6 @@ * successful and FALSE otherwise. re is invalidated regardless of * success. * - * const char *regex_error_desc(struct nhregex *re) - * Used to retrieve an error description from an error created involving - * re. Returns NULL if no description can be retrieved. The returned - * string may be a static buffer and so is only valid until the next - * call to regex_error_desc. - * * boolean regex_match(const char *s, struct nhregex *re) * Used to determine if s (or any substring) matches the regex compiled * into re. Only valid if the most recent call to regex_compile on re @@ -42,6 +36,17 @@ * * void regex_free(struct nhregex *re) * Deallocate a regex object. + * + * char *regex_error_desc(struct nhregex *re, char *outbuf) + * Used to retrieve an explanation for an error encountered by + * regex_compile() or regex_match(). The explanation is copied into + * outbuf[] and a pointer to that is returned. regex_error_desc() + * should be called before regex_free() but outbuf[] may be used after + * that. outbuf[] must be able to hold at least BUFSZ characters. + * + * One possible error result is "out of memory" so freeing the failed + * re should be done to try to recover memory before issuing any error + * feedback. */ const char regex_id[] = "posixregex"; @@ -54,7 +59,7 @@ struct nhregex { struct nhregex * regex_init(void) { - return (struct nhregex *) alloc(sizeof(struct nhregex)); + return (struct nhregex *) alloc(sizeof (struct nhregex)); } boolean @@ -67,19 +72,20 @@ regex_compile(const char *s, struct nhregex *re) return TRUE; } -const char * -regex_error_desc(struct nhregex *re) +char * +regex_error_desc(struct nhregex *re, char *errbuf) { - static char buf[BUFSZ]; - - if (!re || !re->err) - return (const char *) 0; - - /* FIXME: Using a static buffer here is not ideal, but avoids memory - * leaks. Consider the allocation more carefully. */ - regerror(re->err, &re->re, buf, BUFSZ); - - return buf; + if (!re) { + Strcpy(errbuf, "no regexp"); + } else if (!re->err) { + Strcpy(errbuf, "no explanation"); + } else { + errbuf[0] = '\0'; + regerror(re->err, &re->re, errbuf, BUFSZ); + if (!errbuf[0]) + Strcpy(errbuf, "unspecified regexp error"); + } + return errbuf; } boolean @@ -104,3 +110,5 @@ regex_free(struct nhregex *re) regfree(&re->re); free(re); } + +/*posixregex.c*/ diff --git a/sys/unix/Makefile.src b/sys/unix/Makefile.src index 16a761f63..8180fef3e 100644 --- a/sys/unix/Makefile.src +++ b/sys/unix/Makefile.src @@ -905,7 +905,7 @@ $(TARGETPFX)winval.o: ../win/X11/winval.c $(HACK_H) ../include/winX.h $(TARGETPFX)tile.o: tile.c $(HACK_H) $(TARGETPFX)winshim.o: ../win/shim/winshim.c $(HACK_H) $(TARGET_CC) $(TARGET_CFLAGS) -c -o $@ ../win/shim/winshim.c -$(TARGETPFX)cppregex.o: ../sys/share/cppregex.cpp +$(TARGETPFX)cppregex.o: ../sys/share/cppregex.cpp $(CONFIG_H) $(TARGET_CXX) $(TARGET_CXXFLAGS) -c -o $@ ../sys/share/cppregex.cpp $(TARGETPFX)qt_bind.o: ../win/Qt/qt_bind.cpp $(HACK_H) ../win/Qt/qt_pre.h \ ../win/Qt/qt_post.h ../win/Qt/qt_bind.h ../win/Qt/qt_main.h \