Hi,

On Fri, 25 Jan 2008, Kirill wrote:

> diff --git a/install-user.reg.in b/install-user.reg.in

I hope you don't mind me commenting so much on your patch, but I think 
that we are getting a good discussion.

BTW do not feel like what I say is set in stone; if you think I'm wrong, 
tell me so, and why.

I just thought about install-user.reg.in looking awfully like install.reg 
(that thought occurred to me because of what I wrote in that load.reg 
snippet in my other mail).  So would it be possible to generate 
install-user.reg from install.reg instead of install-user.reg.in?

That would serve two purposes:

- the patch would be smaller

- it would be harder to get install.reg and install-user.reg out-of-sync.

> diff --git a/systeminfo.c b/systeminfo.c
> index 08429bf..07444d6 100644
> --- a/systeminfo.c
> +++ b/systeminfo.c
> @@ -8,32 +8,45 @@ TCHAR * msys_path(void)
>       static int found_path = 0;
>       HKEY hKey;
>       LONG lRet;
> +     DWORD msysPathLen = MAX_PATH * sizeof(TCHAR);
> 
>       /* Only bother to get it once. */
>       if (found_path)
>               return msysPath;
>       
> -     lRet = RegOpenKeyEx(HKEY_LOCAL_MACHINE,
> +     /* try to find user-specific settings first */
> +     lRet = RegOpenKeyEx(HKEY_CURRENT_USER,
>                           TEXT(GIT_CHEETAH_REG_PATH),
>                           0, KEY_QUERY_VALUE, &hKey);
> -     
> -     if (lRet == ERROR_SUCCESS)
> -     {
> -             DWORD msysPathLen = MAX_PATH * sizeof(TCHAR);
> -             
> +     if (ERROR_SUCCESS == lRet) {

We have the same code here:

>               lRet = RegQueryValueEx(hKey,
>                                      TEXT(GIT_CHEETAH_REG_PATHTOMSYS),
>                                      NULL, NULL,
>                                      (LPBYTE)msysPath,
>                                      &msysPathLen);
>               RegCloseKey(hKey);
> +     }
> 
> -             if (lRet == ERROR_SUCCESS)
> -             {
> -                     found_path = 1;
> -                     return msysPath;
> +     /* if current user does not have the path, try machine-wide */
> +     if (ERROR_SUCCESS != lRet) {
> +             lRet = RegOpenKeyEx(HKEY_LOCAL_MACHINE,
> +                         TEXT(GIT_CHEETAH_REG_PATH),
> +                         0, KEY_QUERY_VALUE, &hKey);
> +             if (ERROR_SUCCESS == lRet) {

... and here:

> +                     lRet = RegQueryValueEx(hKey,
> +                                    TEXT(GIT_CHEETAH_REG_PATHTOMSYS),
> +                                    NULL, NULL,
> +                                    (LPBYTE)msysPath,
> +                                    &msysPathLen);
> +                     RegCloseKey(hKey);

Would it not be better to check if the user key could be found, if not, 
try the system key, and if that was not found, error out.  Otherwise 
proceed to actually read the value.

Hmm?

Thanks,
Dscho

Reply via email to