Re: [PATCH 2/3] msi: Store the current script in the package.

[Hans Leidekker]

On Tue, 2017-09-19 at 00:23 -0500, Zebediah Figura wrote:
> diff --git a/dlls/msi/custom.c b/dlls/msi/custom.c
> index a3e32d9a24..b463ab659c 100644
> --- a/dlls/msi/custom.c
> +++ b/dlls/msi/custom.c
> @@ -1123,24 +1123,6 @@ static UINT HANDLE_CustomType53_54( MSIPACKAGE *package, const WCHAR *source, co
>      return wait_thread_handle( info );
>  }
>
> -static BOOL action_type_matches_script( UINT type, UINT script )
> -{
> - switch (script)
> - {
> - case SCRIPT_NONE:
> -
>    return FALSE;
> - case SCRIPT_INSTALL:
> -
>    return !(type & msidbCustomActionTypeCommit) && !(type & msidbCustomActionTypeRollback);
> - case SCRIPT_COMMIT:
> -
>    return (type & msidbCustomActionTypeCommit);
> - case SCRIPT_ROLLBACK:
> -
>    return (type & msidbCustomActionTypeRollback);
> - default:
> -
>    ERR("unhandled script %u\n", script);
> - }
> - return FALSE;
>
> -}
> -
>
> static UINT defer_custom_action( MSIPACKAGE *package, const WCHAR *action, UINT type )
>  {
>      WCHAR *actiondata = msi_dup_property( package->db, action );
> @@ -1178,7 +1160,7 @@ static UINT defer_custom_action( MSIPACKAGE *package, const WCHAR *action, UINT
>      return ERROR_SUCCESS;
>  }
>
> -UINT ACTION_CustomAction( MSIPACKAGE *package, LPCWSTR action, UINT script )
> +UINT ACTION_CustomAction( MSIPACKAGE *package, LPCWSTR action )
>  {
>      static const WCHAR query[] = {
>          'S','E','L','E','C','T',' ','*',' ','F','R','O','M',' ',
> @@ -1218,7 +1200,7 @@ UINT ACTION_CustomAction( MSIPACKAGE *package, LPCWSTR action, UINT script )
>          if (type & msidbCustomActionTypeNoImpersonate)
>              WARN("msidbCustomActionTypeNoImpersonate not handled\n");
> -
>    if (!action_type_matches_script( type, script ))
> -
>    if (package->script == SCRIPT_NONE)
>    {

This function was called with script other than SCRIPT_NONE before, so this changes
the order of execution. It would need a test, and should be in a separate patch.

[PATCH 2/3] msi: Store the current script in the package.

[Zebediah Figura]

On 09/19/2017 04:34 AM, Hans Leidekker wrote:
> On Tue, 2017-09-19 at 00:23 -0500, Zebediah Figura wrote:
> > diff --git a/dlls/msi/custom.c b/dlls/msi/custom.c
> > index a3e32d9a24..b463ab659c 100644
> > --- a/dlls/msi/custom.c
> > +++ b/dlls/msi/custom.c
> > @@ -1123,24 +1123,6 @@ static UINT HANDLE_CustomType53_54( MSIPACKAGE *package, const WCHAR *source, co
> >       return wait_thread_handle( info );
> >   }
> >
> > -static BOOL action_type_matches_script( UINT type, UINT script )
> > -{
> > - switch (script)
> > - {
> > - case SCRIPT_NONE:
> > -
> >    return FALSE;
> > - case SCRIPT_INSTALL:
> > -
> >    return !(type & msidbCustomActionTypeCommit) && !(type & msidbCustomActionTypeRollback);
> > - case SCRIPT_COMMIT:
> > -
> >    return (type & msidbCustomActionTypeCommit);
> > - case SCRIPT_ROLLBACK:
> > -
> >    return (type & msidbCustomActionTypeRollback);
> > - default:
> > -
> >    ERR("unhandled script %u\n", script);
> > - }
> > - return FALSE;
> >
> > -}
> > - static UINT defer_custom_action( MSIPACKAGE *package, const WCHAR *action, UINT type )
> > {
> >     WCHAR *actiondata = msi_dup_property( package->db, action );
> >
> > @@ -1178,7 +1160,7 @@ static UINT defer_custom_action( MSIPACKAGE *package, const WCHAR *action, UINT
> >       return ERROR_SUCCESS;
> >   }
> >
> > -UINT ACTION_CustomAction( MSIPACKAGE *package, LPCWSTR action, UINT script )
> > +UINT ACTION_CustomAction( MSIPACKAGE *package, LPCWSTR action )
> >   {
> >       static const WCHAR query[] = {
> >           'S','E','L','E','C','T',' ','*',' ','F','R','O','M',' ',
> > @@ -1218,7 +1200,7 @@ UINT ACTION_CustomAction( MSIPACKAGE *package, LPCWSTR action, UINT script )
> >           if (type & msidbCustomActionTypeNoImpersonate)
> >               WARN("msidbCustomActionTypeNoImpersonate not handled\n");
> > -
> >    if (!action_type_matches_script( type, script ))
> > -
> >    if (package->script == SCRIPT_NONE)
> >     {
>
> This function was called with script other than SCRIPT_NONE before, so this changes
> the order of execution. It would need a test, and should be in a separate patch.

Right, a script supplied by execute_script(). With this patch that 
function just sets package->script itself, so it shouldn't result in any 
change, unless I'm missing something.

[PATCH 2/3] msi: Store the current script in the package.

[Hans Leidekker]

On Tue, 2017-09-19 at 09:27 -0500, Zebediah Figura wrote:
> On 09/19/2017 04:34 AM, Hans Leidekker wrote:
> > On Tue, 2017-09-19 at 00:23 -0500, Zebediah Figura wrote:
> > > diff --git a/dlls/msi/custom.c b/dlls/msi/custom.c
> > > index a3e32d9a24..b463ab659c 100644
> > > --- a/dlls/msi/custom.c
> > > +++ b/dlls/msi/custom.c
> > > @@ -1123,24 +1123,6 @@ static UINT HANDLE_CustomType53_54( MSIPACKAGE *package, const WCHAR *source, co
> > >       return wait_thread_handle( info );
> > >   }
> > >
> > > -static BOOL action_type_matches_script( UINT type, UINT script )
> > > -{
> > > - switch (script)
> > > - {
> > > - case SCRIPT_NONE:
> > > -
> > >    return FALSE;
> > > - case SCRIPT_INSTALL:
> > > -
> > >    return !(type & msidbCustomActionTypeCommit) && !(type & msidbCustomActionTypeRollback);
> > > - case SCRIPT_COMMIT:
> > > -
> > >    return (type & msidbCustomActionTypeCommit);
> > > - case SCRIPT_ROLLBACK:
> > > -
> > >    return (type & msidbCustomActionTypeRollback);
> > > - default:
> > > -
> > >    ERR("unhandled script %u\n", script);
> > > - }
> > > - return FALSE;
> > >
> > > -}
> > > - static UINT defer_custom_action( MSIPACKAGE *package, const WCHAR *action, UINT type )
> > > {
> > >     WCHAR *actiondata = msi_dup_property( package->db, action );
> > >
> > > @@ -1178,7 +1160,7 @@ static UINT defer_custom_action( MSIPACKAGE *package, const WCHAR *action, UINT
> > >       return ERROR_SUCCESS;
> > >   }
> > >
> > > -UINT ACTION_CustomAction( MSIPACKAGE *package, LPCWSTR action, UINT script )
> > > +UINT ACTION_CustomAction( MSIPACKAGE *package, LPCWSTR action )
> > >   {
> > >       static const WCHAR query[] = {
> > >           'S','E','L','E','C','T',' ','*',' ','F','R','O','M',' ',
> > > @@ -1218,7 +1200,7 @@ UINT ACTION_CustomAction( MSIPACKAGE *package, LPCWSTR action, UINT script )
> > >           if (type & msidbCustomActionTypeNoImpersonate)
> > >               WARN("msidbCustomActionTypeNoImpersonate not handled\n");
> > > -
> > >    if (!action_type_matches_script( type, script ))
> > > -
> > >    if (package->script == SCRIPT_NONE)
> > >     {
> >
> > This function was called with script other than SCRIPT_NONE before, so this changes
> > the order of execution. It would need a test, and should be in a separate patch.
>
> Right, a script supplied by execute_script(). With this patch that 
> function just sets package->script itself, so it shouldn't result in any 
> change, unless I'm missing something.

You're right, it's fine.