Re: (try 2)version: check for out of memory when VerInstallFileA/W converstion. (response to coverity 635)

[Rob Shearman]

2008/10/2 Aric Stewart <aric(a)codeweavers.com>:
> @@ -543,34 +543,50 @@ DWORD WINAPI VerInstallFileW(
>          len = WideCharToMultiByte( CP_ACP, 0, srcfilename, -1, NULL, 0, NULL, NULL );
>          if ((wsrcf = HeapAlloc( GetProcessHeap(), 0, len )))
>              WideCharToMultiByte( CP_ACP, 0, srcfilename, -1, wsrcf, len, NULL, NULL );
> +        else
> +            ret = VIF_OUTOFMEMORY;
>      }
> -    if (srcdir)
> +    if (srcdir &&  !ret)

Remove the extra space here.
> +    static const CHAR szSrcDir[] = "C:\\windows\\temp";
> +    static const CHAR szDestDir[] = "C:\\windows\\temp\\test";
> +    static const CHAR szCurDir[] = "C:\\";

This isn't exactly what I meant. C:\Windows might not even exist on
NT4/Win2000 and the temp directory might not be in %WINDIR%\temp. You
should use GetTempPath instead. (Hard-coding C:\ is also something to
be avoided if at all possible.)

-- 
Rob Shearman

(try 2)version: check for out of memory when VerInstallFileA/W converstion. (response to coverity 635)

[Aric Stewart]

Whoops on the whitespace.

The paths are irrelevent.  They are intentionally pointing at a file and 
location that is not existing.  Should I change the paths to express 
that more explicitly;  something like: "D:\\oes\\not\\exist" ?

-aric

Rob Shearman wrote:
> 2008/10/2 Aric Stewart <aric(a)codeweavers.com>:
> > @@ -543,34 +543,50 @@ DWORD WINAPI VerInstallFileW(
> >          len = WideCharToMultiByte( CP_ACP, 0, srcfilename, -1, NULL, 0, NULL, NULL );
> >          if ((wsrcf = HeapAlloc( GetProcessHeap(), 0, len )))
> >              WideCharToMultiByte( CP_ACP, 0, srcfilename, -1, wsrcf, len, NULL, NULL );
> > +        else
> > +            ret = VIF_OUTOFMEMORY;
> >      }
> > -    if (srcdir)
> > +    if (srcdir &&  !ret)
>
> Remove the extra space here.
> > +    static const CHAR szSrcDir[] = "C:\\windows\\temp";
> > +    static const CHAR szDestDir[] = "C:\\windows\\temp\\test";
> > +    static const CHAR szCurDir[] = "C:\\";
>
> This isn't exactly what I meant. C:\Windows might not even exist on
> NT4/Win2000 and the temp directory might not be in %WINDIR%\temp. You
> should use GetTempPath instead. (Hard-coding C:\ is also something to
> be avoided if at all possible.)

(try 2)version: check for out of memory when VerInstallFileA/W converstion. (response to coverity 635)

[Rob Shearman]

2008/10/2 Aric Stewart <aric(a)codeweavers.com>:
> Whoops on the whitespace.
>
> The paths are irrelevent.  They are intentionally pointing at a file and
> location that is not existing.  Should I change the paths to express that
> more explicitly;  something like: "D:\\oes\\not\\exist" ?

Sure, that would work. That way we can be sure it won't give us
different results on different test machines.
> Rob Shearman wrote:
> > 2008/10/2 Aric Stewart <aric(a)codeweavers.com>:
> > > @@ -543,34 +543,50 @@ DWORD WINAPI VerInstallFileW(
> > >         len = WideCharToMultiByte( CP_ACP, 0, srcfilename, -1, NULL, 0,
> > > NULL, NULL );
> > >         if ((wsrcf = HeapAlloc( GetProcessHeap(), 0, len )))
> > >             WideCharToMultiByte( CP_ACP, 0, srcfilename, -1, wsrcf, len,
> > > NULL, NULL );
> > > +        else
> > > +            ret = VIF_OUTOFMEMORY;
> > >     }
> > > -    if (srcdir)
> > > +    if (srcdir &&  !ret)
> >
> > Remove the extra space here.
> > > +    static const CHAR szSrcDir[] = "C:\\windows\\temp";
> > > +    static const CHAR szDestDir[] = "C:\\windows\\temp\\test";
> > > +    static const CHAR szCurDir[] = "C:\\";
> >
> > This isn't exactly what I meant. C:\Windows might not even exist on
> > NT4/Win2000 and the temp directory might not be in %WINDIR%\temp. You
> > should use GetTempPath instead. (Hard-coding C:\ is also something to
> > be avoided if at all possible.)

-- 
Rob Shearman