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 <hack.h>' that 'make depend' was
ignoring.

diff --git a/include/extern.h b/include/extern.h
index 9fc3bb92968b93a412d922760848131d38905b84..711a0d62162aa270f4318953afd5c9a1a98ecefa 100644 (file)
--- 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 1acff8f3f56eeb04d017515808597490bdce9aa6..07fe3598b14d06a9e69213a1df8e8216a6dbfeee 100644 (file)
--- 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 9df6ad57291e28e97712389077e0966df21780a3..bb4f627465e65431602e7c02188a014ad3046773 100644 (file)
--- 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 0e3a7de1a923d16a3c25b827a5c7f14a85f91c46..3f2cbe6224930b074896494e8b1e42c24032d233 100644 (file)
--- 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 <hack.h>
+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<std::regex> re;
     std::unique_ptr<std::regex_error> 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 d04fdacc11bee45128a45d603eb2cbd247e16f7b..0cf477a8c655cb8a705bf8d3ee686d4306f7439d 100644 (file)
--- 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 37f16ada99e359a9d69db506045a789c963e1f43..d6b3c484523a76838757d826a5f0b3f2f7091bab 100644 (file)
--- 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 16a761f63b94dbed45fa4b77acb8a28528653985..8180fef3e71dbdacaf4ce4d1ec649f8fc0aa3dd1 100644 (file)
--- 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 \