Fix cache dir resolving on Windows

Message ID 20190603193716.4861-1-orgads@gmail.com
State Superseded
Headers show
Series
  • Fix cache dir resolving on Windows
Related show

Commit Message

Orgad Shaneh June 3, 2019, 7:37 p.m.
From: Orgad Shaneh <orgads@gmail.com>


... when not running from MSYS environment, and HOME is not set.
---
 gdb/common/pathstuff.c | 4 ++++
 1 file changed, 4 insertions(+)

-- 
2.22.0.rc0.windows.1

Comments

Eli Zaretskii June 4, 2019, 2:44 p.m. | #1
> From: orgads@gmail.com

> Cc: Orgad Shaneh <orgads@gmail.com>

> Date: Mon,  3 Jun 2019 22:37:16 +0300

> 

> From: Orgad Shaneh <orgads@gmail.com>

> 

> ... when not running from MSYS environment, and HOME is not set.

> ---

>  gdb/common/pathstuff.c | 4 ++++

>  1 file changed, 4 insertions(+)

> 

> diff --git a/gdb/common/pathstuff.c b/gdb/common/pathstuff.c

> index 2b1669a5b9..edb8a1fcda 100644

> --- a/gdb/common/pathstuff.c

> +++ b/gdb/common/pathstuff.c

> @@ -231,6 +231,10 @@ get_standard_cache_dir ()

>  #endif

>  

>    const char *home = getenv ("HOME");

> +#ifdef _WIN32

> +  if (home == nullptr)

> +      home = getenv ("USERPROFILE");

> +#endif

>    if (home != NULL)


This is against the MS platform recommendations regarding "known
folders", see

  https://docs.microsoft.com/en-us/windows/desktop/shell/knownfolderid
  https://docs.microsoft.com/en-us/windows/desktop/shell/csidl

But maybe we don't care about that.  I know that many applications
ported from Unix don't.
Simon Marchi June 4, 2019, 3:05 p.m. | #2
On 2019-06-04 10:44 a.m., Eli Zaretskii wrote:
>> From: orgads@gmail.com

>> Cc: Orgad Shaneh <orgads@gmail.com>

>> Date: Mon,  3 Jun 2019 22:37:16 +0300

>>

>> From: Orgad Shaneh <orgads@gmail.com>

>>

>> ... when not running from MSYS environment, and HOME is not set.

>> ---

>>  gdb/common/pathstuff.c | 4 ++++

>>  1 file changed, 4 insertions(+)

>>

>> diff --git a/gdb/common/pathstuff.c b/gdb/common/pathstuff.c

>> index 2b1669a5b9..edb8a1fcda 100644

>> --- a/gdb/common/pathstuff.c

>> +++ b/gdb/common/pathstuff.c

>> @@ -231,6 +231,10 @@ get_standard_cache_dir ()

>>  #endif

>>  

>>    const char *home = getenv ("HOME");

>> +#ifdef _WIN32

>> +  if (home == nullptr)

>> +      home = getenv ("USERPROFILE");

>> +#endif

>>    if (home != NULL)

> 

> This is against the MS platform recommendations regarding "known

> folders", see

> 

>   https://docs.microsoft.com/en-us/windows/desktop/shell/knownfolderid

>   https://docs.microsoft.com/en-us/windows/desktop/shell/csidl

> 

> But maybe we don't care about that.  I know that many applications

> ported from Unix don't.


If it's not too complicated to do The Right Thing (use these  and be a good citizen,
I would prefer that.  But I won't be the one writing the change for Windows
and maintaining it, so I won't force it either.

Simon
Eli Zaretskii June 4, 2019, 4:17 p.m. | #3
> Cc: gdb-patches@sourceware.org

> From: Simon Marchi <simark@simark.ca>

> Date: Tue, 4 Jun 2019 11:05:52 -0400

> 

> >   https://docs.microsoft.com/en-us/windows/desktop/shell/knownfolderid

> >   https://docs.microsoft.com/en-us/windows/desktop/shell/csidl

> > 

> > But maybe we don't care about that.  I know that many applications

> > ported from Unix don't.

> 

> If it's not too complicated to do The Right Thing (use these  and be a good citizen,

> I would prefer that.  But I won't be the one writing the change for Windows

> and maintaining it, so I won't force it either.


If we want to follow the recommendations, it's just the matter of
using a different environment variable: APPDATA instead of
USERPROFILE.  It is usually a subdirectory of USERPROFILE.

Patch

diff --git a/gdb/common/pathstuff.c b/gdb/common/pathstuff.c
index 2b1669a5b9..edb8a1fcda 100644
--- a/gdb/common/pathstuff.c
+++ b/gdb/common/pathstuff.c
@@ -231,6 +231,10 @@  get_standard_cache_dir ()
 #endif
 
   const char *home = getenv ("HOME");
+#ifdef _WIN32
+  if (home == nullptr)
+      home = getenv ("USERPROFILE");
+#endif
   if (home != NULL)
     {
       /* Make sure the path is absolute and tilde-expanded.  */