Re: advpack: Fix buffer sizes for possibly quoted strings (try 2, including testcase)

[James Hawkins]

On Thu, May 29, 2008 at 9:20 AM, Michael Karcher
<wine(a)mkarcher.dialup.fu-berlin.de> wrote:
> Increase buffer size to make quoted strings fit. This fixes the problem
> Dan described in
>  http://www.winehq.org/pipermail/wine-devel/2008-May/065954.html which
> is caused by quoted 'HKLM' not fitting into prefix. This patch does, on
> request, *not* fix the bigger picture issue that made it possible for
> this bug to slip for such a long time, which is missing error checking
> on SetupGetStringFieldW calls in get_dest_dir. Valid .inf files should,
> of course, not exercise the missing error check, so on valid .inf
> files, this patch is sufficient for correct behaviour.
>
> In the testcase with quoted 'HKLM', SetupGetStringFieldW fails because
> of unsufficient buffer size, which is *ignore* by the current code. As
> from the previous destination parsed in the patch the local automatic
> variable still contains the unquoted HKLM and by luck does not get
> overwritten between invocations of get_dest_dir, the testcase passes
> nevertheless except if compiled by gutsy's gcc 4.1 without optimization
> which puts the local variable to a stack location that does not survive
> between different invocations of get_dest_dir.
>
> The attached change to the testcase "pollutes" the local variable prefix
> with HKCU, so the bug is always exposed and not just by bad luck.
> ---
>  dlls/advpack/advpack.c |   10 +++++-----
>  1 files changed, 5 insertions(+), 5 deletions(-)

You must've attached the wrong patch because there is no test case.
Also, don't make a change to the existing tests.  Please write a
completely new test that exposes just the one bug you're trying to
fix.  They're called 'unit' tests for a reason.

-- 
James Hawkins