Skip to content

Commit

Permalink
[CRT] Always initialize _[w]pgmptr with a *FULL* path to the current …
Browse files Browse the repository at this point in the history
…application.

Otherwise fall back to the computed argv[0].
This is expected by some applications, for example Git.
Code is adapted from Wine.

Many thanks to Stanislav Motylkov for having investigated this bug!

CORE-12931 CORE-13892 CORE-13898 CORE-14066
  • Loading branch information
HBelusca committed Jun 5, 2018
1 parent cac1bc1 commit f215f39
Showing 1 changed file with 32 additions and 7 deletions.
39 changes: 32 additions & 7 deletions sdk/lib/crt/misc/getargs.c
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ void __getmainargs(int* argc, char*** argv, char*** env, int expand_wildcards, i
len = strlen(_acmdln);
buffer = malloc(sizeof(char) * len);

// Reference: https://msdn.microsoft.com/en-us/library/a1y7w461(v=vs.71).aspx
// Reference: https://msdn.microsoft.com/en-us/library/a1y7w461.aspx
while (TRUE)
{
// Arguments are delimited by white space, which is either a space or a tab.
Expand Down Expand Up @@ -294,7 +294,6 @@ void __getmainargs(int* argc, char*** argv, char*** env, int expand_wildcards, i

/* Free the temporary buffer. */
free(buffer);
HeapValidate(GetProcessHeap(), 0, NULL);

*argc = __argc;
if (__argv == NULL)
Expand All @@ -304,7 +303,21 @@ void __getmainargs(int* argc, char*** argv, char*** env, int expand_wildcards, i
}
*argv = __argv;
*env = _environ;
_pgmptr = _strdup(__argv[0]);

_pgmptr = malloc(MAX_PATH * sizeof(char));
if (_pgmptr)
{
if (!GetModuleFileNameA(NULL, _pgmptr, MAX_PATH))
_pgmptr[0] = '\0';
else
_pgmptr[MAX_PATH - 1] = '\0';
}
else
{
_pgmptr = _strdup(__argv[0]);

This comment has been minimized.

Copy link
@avsej

avsej Jun 25, 2018

@HBelusca, so malloc might return NULL (I guess if it is not able to allocate memory), but you expect _strdup to succeed and do not check what it returns?

As I understand _strdup is a wrapper for malloc+strcpy:

char *_strdup(const char *_s)
{
char *rv;
if (_s == 0)
return 0;
rv = (char *)malloc(strlen(_s) + 1);
if (rv == 0)
return 0;
strcpy(rv, _s);
return rv;
}

Why this way to handle malloc is sufficient? Looks like it would make future debugging even harder.

This comment has been minimized.

Copy link
@ThFabba

ThFabba Jun 25, 2018

Member

Agreed, this else block doesn't make a whole lot of sense.

This comment has been minimized.

Copy link
@HBelusca

HBelusca Jun 25, 2018

Author Contributor

If length of __argv[0] is smaller than MAX_PATH, then there is a slight chance that the first malloc has failed due to lack of memory but the _strdup will succeed, and _pgmptr will be able to still have "something" (non-empty) in it.
Then of course, if also that _strdup fails, then we stop there and call it a day, _pgmptr will be NULL and that's it.

}

HeapValidate(GetProcessHeap(), 0, NULL);

// if (new_mode) _set_new_mode(*new_mode);
}
Expand Down Expand Up @@ -342,7 +355,7 @@ void __wgetmainargs(int* argc, wchar_t*** wargv, wchar_t*** wenv,
len = wcslen(_wcmdln);
buffer = malloc(sizeof(wchar_t) * len);

// Reference: https://msdn.microsoft.com/en-us/library/a1y7w461(v=vs.71).aspx
// Reference: https://msdn.microsoft.com/en-us/library/a1y7w461.aspx
while (TRUE)
{
// Arguments are delimited by white space, which is either a space or a tab.
Expand Down Expand Up @@ -429,8 +442,6 @@ void __wgetmainargs(int* argc, wchar_t*** wargv, wchar_t*** wenv,
/* Free the temporary buffer. */
free(buffer);

HeapValidate(GetProcessHeap(), 0, NULL);

*argc = __argc;
if (__wargv == NULL)
{
Expand All @@ -439,7 +450,21 @@ void __wgetmainargs(int* argc, wchar_t*** wargv, wchar_t*** wenv,
}
*wargv = __wargv;
*wenv = __winitenv;
_wpgmptr = _wcsdup(__wargv[0]);

_wpgmptr = malloc(MAX_PATH * sizeof(wchar_t));
if (_wpgmptr)
{
if (!GetModuleFileNameW(NULL, _wpgmptr, MAX_PATH))
_wpgmptr[0] = '\0';
else
_wpgmptr[MAX_PATH - 1] = '\0';
}
else
{
_wpgmptr = _wcsdup(__wargv[0]);
}

HeapValidate(GetProcessHeap(), 0, NULL);

// if (new_mode) _set_new_mode(*new_mode);
}
Expand Down

0 comments on commit f215f39

Please sign in to comment.