Commit 5927ed8d authored by Nick Mathewson's avatar Nick Mathewson 🥔
Browse files

checkSpace.pl now forbids more identifiers. 

The functions it warns about are:
  assert, memcmp, strcat, strcpy, sprintf, malloc, free, realloc,
  strdup, strndup, calloc.

Also, fix a few lingering instances of these in the code. Use other
conventions to indicate _intended_ use of assert and
malloc/realloc/etc.
parent 4e3f9c1f

scripts/maint/checkSpace.pl

+19 −0
Original line number Diff line number Diff line
@@ -156,6 +156,25 @@ for $fn (@ARGV) { 
                    $in_func_head = 0;

                }

            }



	    ## Check for forbidden functions except when they are

	    # explicitly permitted

	    if (/\bassert\(/ && not /assert OK/) {

		print "assert :$fn:$.   (use tor_assert)\n";

	    }

	    if (/\bmemcmp\(/ && not /memcmp OK/) {

		print "memcmp :$fn:$.   (use {tor,fast}_mem{eq,neq,cmp}\n";

	    }

	    # always forbidden.

	    if (not / OVERRIDE /) {

		if (/\bstrcat\(/ or /\bstrcpy\(/ or /\bsprintf\(/) {

		    print "$& :$fn:$.\n";

		}

		if (/\bmalloc\(/ or /\bfree\(/ or /\brealloc\(/ or

		    /\bstrdup\(/ or /\bstrndup\(/ or /\bcalloc\(/) {

		    print "$& :$fn:$.    (use tor_malloc, tor_free, etc)\n";

		}

	    }

        }

    }

    ## Warn if the file doesn't end with a blank line.

src/common/backtrace.c

+2 −2
Original line number Diff line number Diff line
@@ -117,7 +117,7 @@ log_backtrace(int severity, int domain, const char *msg) 
  for (i=0; i < depth; ++i) {

    tor_log(severity, domain, "    %s", symbols[i]);

  }

  free(symbols);

  raw_free(symbols);



 done:

  tor_mutex_release(&cb_buf_mutex);
@@ -190,7 +190,7 @@ install_bt_handler(void) 
    size_t depth = backtrace(cb_buf, MAX_DEPTH);

    symbols = backtrace_symbols(cb_buf, (int) depth);

    if (symbols)

      free(symbols);

      raw_free(symbols);

  }



  return rv;

src/common/compat.c

+1 −1
Original line number Diff line number Diff line
@@ -2350,7 +2350,7 @@ make_path_absolute(char *fname) 
  /* We don't want to assume that tor_free can free a string allocated

   * with malloc.  On failure, return fname (it's better than nothing). */

  char *absfname = tor_strdup(absfname_malloced ? absfname_malloced : fname);

  if (absfname_malloced) free(absfname_malloced);

  if (absfname_malloced) raw_free(absfname_malloced);



  return absfname;

#else

src/common/container.h

+1 −1
Original line number Diff line number Diff line
@@ -526,7 +526,7 @@ void* strmap_remove_lc(strmap_t *map, const char *key); 
    return (valtype*)digestmap_remove((digestmap_t*)map, key);          \

  }                                                                     \

  ATTR_UNUSED static inline void                                        \

  prefix##free(maptype *map, void (*free_val)(void*))                   \

  prefix##f##ree(maptype *map, void (*free_val)(void*))                 \

  {                                                                     \

    digestmap_free((digestmap_t*)map, free_val);                        \

  }                                                                     \

src/common/log.c

+9 −7
Original line number Diff line number Diff line
@@ -47,6 +47,8 @@ 
#define TRUNCATED_STR_LEN 14

/** @} */



#define raw_assert(x) assert(x) // assert OK



/** Information for a single logfile; only used in log.c */

typedef struct logfile_t {

  struct logfile_t *next; /**< Next logfile_t in the linked list. */
@@ -75,7 +77,7 @@ sev_to_string(int severity) 
    case LOG_ERR:     return "err";

    default:          /* Call assert, not tor_assert, since tor_assert

                       * calls log on failure. */

                      assert(0); return "UNKNOWN"; // LCOV_EXCL_LINE

                      raw_assert(0); return "UNKNOWN"; // LCOV_EXCL_LINE

  }

}
@@ -95,7 +97,7 @@ should_log_function_name(log_domain_mask_t domain, int severity) 
      return (domain & (LD_BUG|LD_NOFUNCNAME)) == LD_BUG;

    default:

      /* Call assert, not tor_assert, since tor_assert calls log on failure. */

      assert(0); return 0; // LCOV_EXCL_LINE

      raw_assert(0); return 0; // LCOV_EXCL_LINE

  }

}
@@ -293,7 +295,7 @@ format_msg(char *buf, size_t buf_len, 
  char *end_of_prefix;

  char *buf_end;



  assert(buf_len >= 16); /* prevent integer underflow and general stupidity */

  raw_assert(buf_len >= 16); /* prevent integer underflow and stupidity */

  buf_len -= 2; /* subtract 2 characters so we have room for \n\0 */

  buf_end = buf+buf_len; /* point *after* the last char we can write to */
@@ -482,12 +484,12 @@ logv,(int severity, log_domain_mask_t domain, const char *funcname, 
  int callbacks_deferred = 0;



  /* Call assert, not tor_assert, since tor_assert calls log on failure. */

  assert(format);

  raw_assert(format);

  /* check that severity is sane.  Overrunning the masks array leads to

   * interesting and hard to diagnose effects */

  assert(severity >= LOG_ERR && severity <= LOG_DEBUG);

  raw_assert(severity >= LOG_ERR && severity <= LOG_DEBUG);

  /* check that we've initialised the log mutex before we try to lock it */

  assert(log_mutex_initialized);

  raw_assert(log_mutex_initialized);

  LOCK_LOGS();



  if ((! (domain & LD_NOCB)) && pending_cb_messages
@@ -658,7 +660,7 @@ tor_log_update_sigsafe_err_fds(void) 
  if (!found_real_stderr &&

      int_array_contains(sigsafe_log_fds, n_sigsafe_log_fds, STDOUT_FILENO)) {

    /* Don't use a virtual stderr when we're also logging to stdout. */

    assert(n_sigsafe_log_fds >= 2); /* Don't use assert inside log functions*/

    raw_assert(n_sigsafe_log_fds >= 2); /* Don't tor_assert inside log fns */

    sigsafe_log_fds[0] = sigsafe_log_fds[--n_sigsafe_log_fds];

  }