[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH for-1.4] tests/test-string-input-visitor: Handle
From: |
Andreas Färber |
Subject: |
Re: [Qemu-devel] [PATCH for-1.4] tests/test-string-input-visitor: Handle errors provoked by fuzz test |
Date: |
Sat, 02 Feb 2013 22:37:22 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130105 Thunderbird/17.0.2 |
Am 02.02.2013 22:19, schrieb Peter Maydell:
> It's OK and expected for visitors to return errors when presented with
> the fuzz test's random data. This means the test harness needs to
> handle them; check for and free any error after each visitor call,
> and only free the string returned by visit_type_str if visit_type_str
> succeeded.
>
> This fixes a problem where this test failed the MacOSX malloc()
> consistency checks and might segfault on other platforms [due
> to calling free() on an uninitialized pointer variable].
>
> Signed-off-by: Peter Maydell <address@hidden>
> ---
> tests/test-string-input-visitor.c | 23 ++++++++++++++++++++++-
> 1 file changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/tests/test-string-input-visitor.c
> b/tests/test-string-input-visitor.c
> index f6b0093..793b334 100644
> --- a/tests/test-string-input-visitor.c
> +++ b/tests/test-string-input-visitor.c
> @@ -194,20 +194,41 @@ static void test_visitor_in_fuzz(TestInputVisitorData
> *data,
>
> v = visitor_input_test_init(data, buf);
> visit_type_int(v, &ires, NULL, &errp);
> + if (error_is_set(&errp)) {
> + error_free(errp);
> + errp = NULL;
> + }
It seems to me the naming is bad here: errp appears to be an Error*, not
an Error**. It would be nice to fix this within the function touched.
Since it is an Error*, I think it was said that we should not use
error_is_set() but err != NULL (or if you prefer, just err).
error_is_set() is intended for **errp arguments that may be NULL.
Your analysis and the freeing and NULL'ing as solution looks correct.
Andreas
>
> v = visitor_input_test_init(data, buf);
> visit_type_bool(v, &bres, NULL, &errp);
> + if (error_is_set(&errp)) {
> + error_free(errp);
> + errp = NULL;
> + }
> visitor_input_teardown(data, NULL);
>
> v = visitor_input_test_init(data, buf);
> visit_type_number(v, &nres, NULL, &errp);
> + if (error_is_set(&errp)) {
> + error_free(errp);
> + errp = NULL;
> + }
>
> v = visitor_input_test_init(data, buf);
> visit_type_str(v, &sres, NULL, &errp);
> - g_free(sres);
> + if (error_is_set(&errp)) {
> + error_free(errp);
> + errp = NULL;
> + } else {
> + g_free(sres);
> + }
>
> v = visitor_input_test_init(data, buf);
> visit_type_EnumOne(v, &eres, NULL, &errp);
> + if (error_is_set(&errp)) {
> + error_free(errp);
> + errp = NULL;
> + }
> visitor_input_teardown(data, NULL);
> }
> }
>
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg