Re: msi: Fix crash with access to component

[Hans Leidekker]

On Saturday 30 January 2010 12:55:21 André Hentschel wrote:
> dlls/msi/files.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/dlls/msi/files.c b/dlls/msi/files.c
> index 7e74dca..afeb451 100644
> --- a/dlls/msi/files.c
> +++ b/dlls/msi/files.c
> @@ -363,6 +363,8 @@ static UINT ITERATE_DuplicateFiles(MSIRECORD *row, LPVOID param)
>      {
>          TRACE("Skipping copy due to disabled component %s\n",
>                          debugstr_w(component));
> -
>    if (!comp)
> -
>        return ERROR_INSTALL_FAILURE;
>
>    /* the action taken was the same as the current install state */        
>    comp->Action = comp->Installed;

Where do you see that crash? Are you sure ERROR_INSTALL_FAILURE is appropriate?

-Hans

msi: Fix crash with access to component

[André Hentschel]

Hans Leidekker schrieb:
> On Saturday 30 January 2010 12:55:21 André Hentschel wrote:
> > dlls/msi/files.c |    2 ++
> >  1 files changed, 2 insertions(+), 0 deletions(-)
> >
> > diff --git a/dlls/msi/files.c b/dlls/msi/files.c
> > index 7e74dca..afeb451 100644
> > --- a/dlls/msi/files.c
> > +++ b/dlls/msi/files.c
> > @@ -363,6 +363,8 @@ static UINT ITERATE_DuplicateFiles(MSIRECORD *row, LPVOID param)
> >      {
> >          TRACE("Skipping copy due to disabled component %s\n",
> >                          debugstr_w(component));
> > -
> >    if (!comp)
> > -
> >        return ERROR_INSTALL_FAILURE;
> >
> >    /* the action taken was the same as the current install state */        
> >    comp->Action = comp->Installed;
>
> Where do you see that crash? Are you sure ERROR_INSTALL_FAILURE is appropriate?
>
> -Hans

thought twice(and digging into the code some more), maybe returning a success is a better idea.

msi: Fix crash with access to component

[James Hawkins]

2010/1/30 André Hentschel :
> Hans Leidekker schrieb:
> > On Saturday 30 January 2010 12:55:21 André Hentschel wrote:
> > > dlls/msi/files.c |    2 ++
> > >  1 files changed, 2 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/dlls/msi/files.c b/dlls/msi/files.c
> > > index 7e74dca..afeb451 100644
> > > --- a/dlls/msi/files.c
> > > +++ b/dlls/msi/files.c
> > > @@ -363,6 +363,8 @@ static UINT ITERATE_DuplicateFiles(MSIRECORD *row, LPVOID param)
> > >      {
> > >          TRACE("Skipping copy due to disabled component %s\n",
> > >                          debugstr_w(component));
> > > - if (!comp)
> > > - return ERROR_INSTALL_FAILURE;
> > >
> > > /* the action taken was the same as the current install state */
> > >          comp->Action = comp->Installed;
> >
> > Where do you see that crash? Are you sure ERROR_INSTALL_FAILURE is appropriate?
> >
> > -Hans
>
> thought twice(and digging into the code some more), maybe returning a success is a better idea.

And writing a test to determine the correct behavior is an even better idea.