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.

-- 

Best Regards, André Hentschel

msi: Fix crash with access to component

[James Hawkins]

2010/1/30 André Hentschel <nerv(a)dawncrow.de>:
> 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.

-- 
James Hawkins