potential ctype.h issues

A few things which might conceivably pass negative values to ctype
routines.  Some are post-3.6.0.  None of them explain the sporadic
Windows assertion failure.

Using tolower() without verifying the argument isupper() should be
completely safe when tolower() is a function but might not be when
it's a macro.  (Likewise for toupper() without islower().)  NetHack's
lowc() function is always safe, at least for ASCII.

src/makemon.c

@@ -1468,7 +1468,7 @@ rndmonst()
             rndmonst_state.mchoices[mndx] = 0;
             if (tooweak(mndx, minmlev) || toostrong(mndx, maxmlev))
                 continue;
-            if (upper && !isupper(def_monsyms[(int) (ptr->mlet)].sym))
+            if (upper && !isupper((uchar) def_monsyms[(int) ptr->mlet].sym))
                 continue;
             if (elemlevel && wrong_elem_type(ptr))
                 continue;

src/options.c

@@ -587,10 +587,13 @@ boolean val_allowed;
 
         if (!p || (q && q < p))
             p = q;
-        while (p && p > user_string && isspace((uchar) * (p - 1)))
+        if (p) {
-            p--;
+            /* 'user_string' hasn't necessarily been through mungspaces()
-        if (p)
+               so might have tabs or consecutive spaces */
+            while (p > user_string && isspace((uchar) *(p - 1)))
+                p--;
             len = (int) (p - user_string);
+        }
     }
 
     return (boolean) (len >= min_length
@@ -2110,7 +2113,7 @@ boolean tinitial, tfrom_file;
                 bad_negation(fullname, TRUE);
                 return;
             }
-            tmp = tolower(*op);
+            tmp = lowc(*op);
         }
         switch (tmp) {
         case 's': /* single message history cycle (default if negated) */
@@ -2576,30 +2579,24 @@ boolean tinitial, tfrom_file;
             bad_negation(fullname, FALSE);
             return;
         } else if ((op = string_for_env_opt(fullname, opts, FALSE)) != 0) {
-            switch (tolower(*op)) {
+            switch (lowc(*op)) {
-            /* Unencumbered */
+            case 'u': /* Unencumbered */
-            case 'u':
                 flags.pickup_burden = UNENCUMBERED;
                 break;
-            /* Burdened (slight encumbrance) */
+            case 'b': /* Burdened (slight encumbrance) */
-            case 'b':
                 flags.pickup_burden = SLT_ENCUMBER;
                 break;
-            /* streSsed (moderate encumbrance) */
+            case 's': /* streSsed (moderate encumbrance) */
-            case 's':
                 flags.pickup_burden = MOD_ENCUMBER;
                 break;
-            /* straiNed (heavy encumbrance) */
+            case 'n': /* straiNed (heavy encumbrance) */
-            case 'n':
                 flags.pickup_burden = HVY_ENCUMBER;
                 break;
-            /* OverTaxed (extreme encumbrance) */
+            case 'o': /* OverTaxed (extreme encumbrance) */
-            case 'o':
             case 't':
                 flags.pickup_burden = EXT_ENCUMBER;
                 break;
-            /* overLoaded */
+            case 'l': /* overLoaded */
-            case 'l':
                 flags.pickup_burden = OVERLOADED;
                 break;
             default:
@@ -2881,11 +2878,13 @@ boolean tinitial, tfrom_file;
     if (match_optname(opts, fullname, 4, TRUE)) {
         op = string_for_env_opt(fullname, opts, FALSE);
         if (op) {
-            switch (tolower(*op)) {
+            char c = lowc(*op);
-            case 'n':
+
-            case 'l':
+            switch (c) {
-            case 'f':
+            case 'n': /* none */
-                flags.sortloot = tolower(*op);
+            case 'l': /* loot (pickup) */
+            case 'f': /* full (pickup + invent) */
+                flags.sortloot = c;
                 break;
             default:
                 badoption(opts);
@@ -3163,22 +3162,26 @@ boolean tinitial, tfrom_file;
                 return; /* string_for_opt gave feedback */
             tmp = negated ? 'n' : 'f';
         } else {
-            tmp = tolower(*op);
+            tmp = lowc(*op);
         }
         switch (tmp) {
         case 'n': /* none */
-        case 't': /* traditional */
+        case 't': /* traditional: prompt for class(es) by symbol,
+                     prompt for each item within class(es) one at a time */
             flags.menu_style = MENU_TRADITIONAL;
             break;
-        case 'c': /* combo: trad.class sel+menu */
+        case 'c': /* combination: prompt for class(es) by symbol,
+                     choose items within selected class(es) by menu */
             flags.menu_style = MENU_COMBINATION;
             break;
-        case 'p': /* partial: no class menu */
+        case 'f': /* full: choose class(es) by first menu,
-            flags.menu_style = MENU_PARTIAL;
+                     choose items within selected class(es) by second menu */
-            break;
-        case 'f': /* full: class menu + menu */
             flags.menu_style = MENU_FULL;
             break;
+        case 'p': /* partial: skip class filtering,
+                     choose items among all classes by menu */
+            flags.menu_style = MENU_PARTIAL;
+            break;
         default:
             badoption(opts);
         }
@@ -5254,7 +5257,7 @@ const char *strval;
     buf[0] = '\0';
     if (!strval[0] || !strval[1]) { /* empty, or single character */
         /* if single char is space or tab, leave buf[0]=='\0' */
-        if (!isspace(strval[0]))
+        if (!isspace((uchar) strval[0]))
             buf[0] = strval[0];
     } else if (strval[0] == '\'') { /* single quote */
         /* simple matching single quote; we know strval[1] isn't '\0' */