avifil32: Simplify comparing two unsigned ints (PVS-Studio) (try 2)

Ken Thomases ken at codeweavers.com
Mon Nov 10 17:09:15 CST 2014


On Nov 10, 2014, at 4:56 PM, André Hentschel <nerv at dawncrow.de> wrote:

> try 2: added the missing +1 thanks to a hint by ken on irc
> ---
> dlls/avifil32/avifile.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/dlls/avifil32/avifile.c b/dlls/avifil32/avifile.c
> index 6295c7b..3d90e20 100644
> --- a/dlls/avifil32/avifile.c
> +++ b/dlls/avifil32/avifile.c
> @@ -466,9 +466,9 @@ static HRESULT WINAPI IAVIFile_fnDeleteStream(IAVIFile *iface, DWORD fccType, LO
>      /* ... so delete it now */
>      HeapFree(GetProcessHeap(), 0, This->ppStreams[nStream]);
>  
> -    if (This->fInfo.dwStreams - nStream > 0)
> -      memcpy(This->ppStreams + nStream, This->ppStreams + nStream + 1,
> -	     (This->fInfo.dwStreams - nStream) * sizeof(IAVIStreamImpl*));
> +    if (nStream < This->fInfo.dwStreams)
> +      memcpy(&This->ppStreams[nStream], &This->ppStreams[nStream + 1],
> +             (This->fInfo.dwStreams - nStream) * sizeof(This->ppStreams[0]));
>  
>      This->ppStreams[This->fInfo.dwStreams] = NULL;
>      This->fInfo.dwStreams--;

This is a more faithful refactoring of the original logic.  However, looking at it some more, it looks like the original logic was wrong.  It's removing an entry from the array and shifting the following entries up to fill the gap.  There are This->fInfo.dwStreams - nStream entries from the nStream'th one to the end.  There are only This->fInfo.dwStreams - (nStream + 1) entries that actually _follow_ the nStream'th one.  The original logic read past the end of the array and this logic still does.

-Ken




More information about the wine-devel mailing list