Question: I'm working on a fork of Cygwin for native Windows development.

In [this commit](http://www.kylheku.com/cgit/cygnal/commit/?id=8da90498684e5dc90870601dc520d8597e09788d), I'm trying to improve the security of code that spawns `cmd.exe`, used by functions like `system`, `popen`.

What I would like to focus on is this:

```
static const char *cmd_exe_path = NULL;

static void init_cmd_exe_path(void)
{
  char sysdir[NT_MAX_PATH];
  char cmdname[] = "cmd.exe";
  unsigned int nchars;

  if ((nchars = GetSystemDirectoryA(sysdir, sizeof sysdir)) < sizeof sysdir) {
    unsigned int total = nchars + 1 + sizeof cmdname + 1;
    char *path = static_cast<char *>(cmalloc_abort(HEAP_STR, total));
    snprintf(path, total, "%s\\%s", sysdir, cmdname);
    cmd_exe_path = path;
  }
}

const char *get_cmd_exe_path()
{
  static pthread_once_t cmd_exe_once = PTHREAD_ONCE_INIT;
  pthread_once (&cmd_exe_once, init_cmd_exe_path);
  return cmd_exe_path;
}

```

The idea here is to avoid relying on a hard-coded path like `C:\Windows\System32\cmd.exe`, as well as to avoid the `COMSPEC` environment variable which could be commandeered by an attacker. Comment about anything are welcome: API usage, bugs, security concerns, correct use of Cygwin internals (I have no idea whether `HEAP_STR` is the best choice in `cmalloc_abort`).

Oops, I see I used `(void)`, out of habit, on the static function, even though this is C++.

Answer: 
You're allocating too much memory:

```
unsigned int total = nchars + 1 + sizeof cmdname + 1;

```

You only need 1 terminating null at the end of the string. This is already accounted for in the size of `cmdname` (since you declare it as a string initialized `[]`). You can simply allocate:

```
unsigned int total = nchars + 1 + sizeof cmdname;

```

Or, if you add the "\" onto `cmdname` you could do:

```
char cmdname[] = "\\cmd.exe";
unsigned int total = nchars + sizeof cmdname;

```
This belongs in a comment (which I cannot make) but why are you declaring cmd\_exe\_path as const? And why is cmdname hard-declared as an array? It may improve readability if it was a char\*.