Re: [PATCH v3 02/16] fuzz: Add general virtual-device fuzzer

From: Darren Kenny <darren.kenny@oracle.com>
To: Alexander Bulekov <alxndr@bu.edu>
Cc: "Laurent Vivier" <lvivier@redhat.com>,
	"Thomas Huth" <thuth@redhat.com>,
	qemu-devel@nongnu.org, bsd@redhat.com, stefanha@redhat.com,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>
Subject: Re: [PATCH v3 02/16] fuzz: Add general virtual-device fuzzer
Date: Wed, 07 Oct 2020 14:53:40 +0100	[thread overview]
Message-ID: <m27ds2dtwb.fsf@oracle.com> (raw)
In-Reply-To: <20201007133932.ua6tg7hnr3ixpgni@mozz.bu.edu>

On Wednesday, 2020-10-07 at 09:39:32 -04, Alexander Bulekov wrote:
> On 201001 1629, Darren Kenny wrote:

...

>>
>> It might make sense to put the definition of SEPARATOR and some variant
>> of the above the comments in patch 9 where you're adding this related
>> functionality?
>> 
>> It seems a little out of place here.
>> 
>> Thanks,
>> 
>> Darren.
>> 
>
> Hi Darren,
> If I move the definition of SEPARATOR to Patch 9, I would need some
> different way to parse commands here, to keep everything bisectable. I
> don't think the separator is only important in the context of the
> Crossover functionality (Patch 9) - it is useful in general as a
> "stable" way to parse an input into multiple commands.
> Is it OK if I keep SEPARATOR in this patch and add the comments you
> mention to both this patch and patch 9?

Sounds fine, it was just a suggestion since I hadn't seen it being used
in this file, but maybe I missed something.

Thanks,

Darren.

> Thanks
> -Alex
>
>> >> 
>> >> Can you fuzz writing "FUZZ" in memory? Like:
>> >> OP_WRITE(0x100000, "UsingLibFUZZerString")?
>> >
>> > No.. Hopefully that's not a huge problem.
>> >
>> >> > +
>> >> > +enum cmds {
>> >> > +    OP_IN,
>> >> > +    OP_OUT,
>> >> > +    OP_READ,
>> >> > +    OP_WRITE,
>> >> > +    OP_CLOCK_STEP,
>> >> > +};
>> >> > +
>> >> > +#define DEFAULT_TIMEOUT_US 100000
>> >> > +#define USEC_IN_SEC 100000000
>> >> 
>> >> Are you sure this definition is correct?
>> >> 
>> > Thanks for the catch...
>> >
>> >> > +
>> >> > +typedef struct {
>> >> > +    ram_addr_t addr;
>> >> > +    ram_addr_t size; /* The number of bytes until the end of the I/O region */
>> >> > +} address_range;
>> >> > +
>> >> > +static useconds_t timeout = 100000;
>> >> [...]
>> >> 