Re: wineboot: Basic RunOnceEx support

[Dmitry Timoshkov]

"Johan Dahlin" <jdahlin(a)gmail.com> wrote:
> +typedef HRESULT (*DLLREGISTER)          (void);
> +typedef HRESULT (*DLLINSTALL)           (BOOL,LPCWSTR);

Wrong calling convention.
> +static int RunDllRegisterServer(WCHAR* strDll)
> +{
> +    DLLREGISTER pfRegister = NULL;
> +    HRESULT hr;
> +    HMODULE DllHandle = NULL;
> +
> +    pfRegister = LoadProc(strDll, "DllRegisterServer", &DllHandle);
> +
> +    if (!pfRegister)
> +      return -1;

If you've decided to return an HRESULT then use something more appropriate
here instead of -1.
> +    hr = pfRegister();
> +
> +    if(DllHandle)
> +        FreeLibrary(DllHandle);

'if(DllHandle)' check above is redundant.
> +
> +    return hr;
> +}

If you are returning an HRESULT then use an appropriate return type for
the function.
> +static HRESULT RunDllInstall(WCHAR *strDll, BOOL install, WCHAR *command_line)
> +{
> +    DLLINSTALL pfInstall = NULL;
> +    HRESULT hr;
> +    HMODULE DllHandle = NULL;
> +
> +    pfInstall = LoadProc(strDll, "DllInstall", &DllHandle);
> +
> +    if (!pfInstall)
> +      return -1;

Same as above.
> +    hr = pfInstall(install, command_line);
> +
> +    if(DllHandle)
> +        FreeLibrary(DllHandle);

Same as above.
> +static BOOL ProcessRunOnceExKey(HKEY hkRunEntry)
> +{
> +    DWORD i;
> +    DWORD res;
> +    DWORD nMaxCmdLine=0, nMaxValue=0;
> +    WCHAR *szCmdLine=NULL;
> +    WCHAR *szValue=NULL;
> +    static const WCHAR KEY_DLLREGISTERSERVER[]={
> +      'D','l','l','R','e','g','i','s','t','e','r','S','e','r','v','e','r',0};
> +    static const WCHAR KEY_DLLINSTALL[]={
> +      'D','l','l','I','n','s','t','a','l','l',0};
> +
> +    if( (res=RegQueryInfoKeyW( hkRunEntry, NULL, NULL, NULL, NULL, NULL, NULL, &i, &nMaxValue,
> +                    &nMaxCmdLine, NULL, NULL ))!=ERROR_SUCCESS )
> +        goto end;

Why not 'return' isntead of 'goto'?
> +    if( (szCmdLine=HeapAlloc(GetProcessHeap(),0,nMaxCmdLine))==NULL )
> +    {
> +        WINE_ERR("Couldn't allocate memory for the commands to be executed\n");
> +
> +        res=ERROR_NOT_ENOUGH_MEMORY;
> +        goto end;
> +    }

Same here.
> +    if( (szValue=HeapAlloc(GetProcessHeap(),0,(++nMaxValue)*sizeof(*szValue)))==NULL )
> +    {
> +        WINE_ERR("Couldn't allocate memory for the value names\n");
> +
> +        res=ERROR_NOT_ENOUGH_MEMORY;
> +        goto end;
> +    }
> +
> +    while( i>0 )
> +    {
> +        DWORD nValLength=nMaxValue, nDataLength=nMaxCmdLine;
> +        DWORD type;
> +        WCHAR *first, *second, *third; /* | separated parts of the entry */
> +        WCHAR pipe[] = {'|',0};
> +        i--;
> +
> +        if( (res=RegEnumValueW( hkRunEntry, i, szValue, &nValLength, 0, &type,
> +                                (LPBYTE)szCmdLine, &nDataLength ))!=ERROR_SUCCESS )
> +        {
> +            WINE_ERR("Couldn't read in value %d - %d\n", i, res );
> +            break;
> +        }
> +
> +        if (strlenW(szValue) == 0)
> +        {
> +          WINE_TRACE("Processing %s entries\n", wine_dbgstr_w( szCmdLine ));
> +          continue;
> +        }
> +
> +        if( type!=REG_SZ )
> +        {
> +            WINE_ERR("Incorrect type of value #%d (%d)\n", i, type );
> +            continue;
> +        }
> +
> +        if( (res=RegDeleteValueW( hkRunEntry, szValue ))!=ERROR_SUCCESS )
> +        {
> +            WINE_ERR("Couldn't delete value - %d, %d. Running command anyways.\n", i, res );
> +        }
> +
> +        if ((first = strstrW(szCmdLine, pipe)) != NULL)
> +        {
> +          WCHAR *value = first + strlenW(pipe);
> +          HRESULT retval;
> +          if (strncmpW(value, KEY_DLLREGISTERSERVER,
> +                       strlenW(KEY_DLLREGISTERSERVER)) == 0)
> +            {
> +              DWORD len;
> +              WCHAR *cmd;
> +              len = strlenW(szCmdLine) - strlenW(first) + strlenW(pipe);
> +              cmd = HeapAlloc(GetProcessHeap(), 0, sizeof(WCHAR) * len);
> +              if (cmd == NULL)
> +                goto end;
> +
> +              if (lstrcpynW(cmd, szCmdLine, len))
> +              {
> +                  WINE_TRACE("DllRegisterServer %s\n", wine_dbgstr_w(cmd));
> +                  retval = RunDllRegisterServer(cmd);
> +                  WINE_TRACE("DllRegisterServer returned %d\n", retval);
> +              }
> +              HeapFree(GetProcessHeap(), 0, cmd);
> +            }
> +          else if (strncmpW(value, KEY_DLLINSTALL,
> +                            strlenW(KEY_DLLINSTALL)) == 0)
> +            {
> +              WCHAR *filename, *arg = NULL;
> +              WCHAR install = FALSE;
> +              HRESULT retval;
> +              DWORD filenameLength, argLength;
> +
> +              /* filename|DllInstall
> +               * filename|DllInstall|i,arg
> +               * filename|DllInstall|I,arg
> +               * FIXME: is i=FALSE,I=TRUE or the reverse?
> +               */
> +              second = strstrW(first + strlenW(pipe), pipe);
> +              if (second == NULL)
> +                filenameLength = strlenW(szCmdLine) - strlenW(first) + strlenW(pipe);
> +              else if ((third = strstrW(second, pipe)) != NULL)
> +                {
> +                  filenameLength = strlenW(szCmdLine) - strlenW(first) + strlenW(pipe);
> +                  install = *(second + strlenW(pipe));
> +
> +                  argLength = strlenW(third) - strlenW(pipe) - 1;
> +                  filename = HeapAlloc(GetProcessHeap(), 0, sizeof(WCHAR) * argLength);
> +                  lstrcpynW(arg, third + strlenW(pipe) + 2, argLength);
> +                }
> +
> +              filename = HeapAlloc(GetProcessHeap(), 0, sizeof(WCHAR) * filenameLength);
> +              lstrcpynW(filename, szCmdLine, filenameLength);
> +
> +              WINE_TRACE("DllInstall (%s, %d, %s)\n",
> +                         wine_dbgstr_w(filename),
> +                         install == 'i',
> +                         wine_dbgstr_w(arg));
> +              retval = RunDllInstall(filename, install == 'i', arg);
> +              WINE_TRACE("DllInstall returned %d\n", retval);
> +              HeapFree( GetProcessHeap(), 0, filename );
> +              if (arg)
> +                HeapFree( GetProcessHeap(), 0, arg );
> +            }
> +        } else {
> +          if( (res=runCmd(szCmdLine, NULL, TRUE, FALSE ))==INVALID_RUNCMD_RETURN )
> +          {
> +              WINE_ERR("Error running cmd #%d (%d)\n", i, GetLastError() );
> +          }
> +        }
> +    }
> + end:
> +    HeapFree( GetProcessHeap(), 0, szValue );
> +    HeapFree( GetProcessHeap(), 0, szCmdLine );
> +
> +    return res;
> +}

You are returning 'res' while using 'retval' in some code paths.

-- 
Dmitry.