Re: msacm32: Fix greater than vs. less than typo in comparison. (Coverity)

[Alex Villací­s Lasso]

Kai Blin escribió:
> ---
>  dlls/msacm32/driver.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> ------------------------------------------------------------------------
>
> diff --git a/dlls/msacm32/driver.c b/dlls/msacm32/driver.c
> index d4ad644..06e6614 100644
> --- a/dlls/msacm32/driver.c
> +++ b/dlls/msacm32/driver.c
> @@ -430,7 +430,7 @@ LRESULT WINAPI acmDriverMessage(HACMDRIV
>                  /* This verification is required because DRVCONFIGINFO is 12 bytes
>                     long, yet native msacm reports a 16-byte structure to codecs.
>                   */
> -                if (iStructSize < sizeof(DRVCONFIGINFO)) iStructSize = sizeof(DRVCONFIGINFO);
> +                if (iStructSize > sizeof(DRVCONFIGINFO)) iStructSize = sizeof(DRVCONFIGINFO);
>                  pConfigInfo = HeapAlloc(MSACM_hHeap, 0, iStructSize);
>                  if (!pConfigInfo) {
>                      ERR("OOM while supplying DRVCONFIGINFO for DRV_CONFIGURE, using NULL\n");
>
> ------------------------------------------------------------------------

This is not a typo, but rather an unnecessary verification, from when I 
didn't know the true size of a DRVCONFIGINFO. The struct size should be 
*exactly 16 bytes*, even though only the first 12 bytes are used. This 
patch might even introduce a bug, where the struct info assumed by the 
codec to be 16 bytes when only 12 are allocated, and some smart-ass 
codec tries to use the last 4 bytes of the struct for its own purposes, 
which with this patch are now past the end of the allocated array. A 
more correct patch would fix the comment to indicate that exactly 16 
bytes are allocated for a 12-byte struct, and remove the conditional 
assignment altogether.

Alex Villacís Lasso

-- 
perl -e '$x = 2.4; print sprintf("%.0f + %.0f = %.0f\n", $x, $x, $x + $x);'

msacm32: Fix greater than vs. less than typo in comparison. (Coverity)

[Kai Blin]

On Friday 05 January 2007 19:00, Alex Villací­s Lasso wrote:
> This is not a typo, but rather an unnecessary verification, from when I
> didn't know the true size of a DRVCONFIGINFO. The struct size should be
> *exactly 16 bytes*, even though only the first 12 bytes are used. This
> patch might even introduce a bug, where the struct info assumed by the
> codec to be 16 bytes when only 12 are allocated, and some smart-ass
> codec tries to use the last 4 bytes of the struct for its own purposes,
> which with this patch are now past the end of the allocated array. A
> more correct patch would fix the comment to indicate that exactly 16
> bytes are allocated for a 12-byte struct, and remove the conditional
> assignment altogether.

Oh, ok. Looking at the code and the comment, I wasn't sure which of that was 
the case, and sent the patch hoping for this kind of response. I'll fix it 
then.

Thanks,
Kai

-- 
Kai Blin, <kai Dot blin At gmail Dot com>
WorldForge developer    http://www.worldforge.org/
Wine developer          http://wiki.winehq.org/KaiBlin/
--
Will code for cotton.