Re: [Qemu-devel] [PATCH 4/4] kvm: i386: Add classic PCI device assignment

[Blue Swirl]

On Tue, Aug 28, 2012 at 9:51 PM, Anthony Liguori <address@hidden> wrote:
> Blue Swirl <address@hidden> writes:
>
>> On Tue, Aug 28, 2012 at 7:31 PM, Anthony Liguori <address@hidden> wrote:
>>> Blue Swirl <address@hidden> writes:
>>>
>>>> On Tue, Aug 28, 2012 at 5:28 PM, Michael S. Tsirkin <address@hidden> wrote:
>>>>> On Tue, Aug 28, 2012 at 05:01:55PM +0000, Blue Swirl wrote:
>>>>>> On Tue, Aug 28, 2012 at 7:35 AM, Michael Tokarev <address@hidden> wrote:
>>>>>> > On 27.08.2012 22:56, Blue Swirl wrote:
>>>>>> > []
>>>>>> >>> +static uint32_t slow_bar_readb(void *opaque, target_phys_addr_t 
>>>>>> >>> addr)
>>>>>> >>> +{
>>>>>> >>> +    AssignedDevRegion *d = opaque;
>>>>>> >>> +    uint8_t *in = d->u.r_virtbase + addr;
>>>>>> >>
>>>>>> >> Don't perform arithmetic with void pointers.
>>>>>> >
>>>>>> > There are a few places in common qemu code which does this for a very
>>>>>> > long time.  So I guess it is safe now.
>>>>>>
>>>>>> It's a non-standard GCC extension.
>>>>>
>>>>> So?  We use many other GCC extensions. grep for typeof.
>>>>
>>>> Dependencies should not be introduced trivially. In this case, it's
>>>> pretty easy to avoid void pointer arithmetic as Jan's next version
>>>> shows.
>>>
>>> The standard is vague with respect void arithmetic.  Most compilers
>>> allow it.  A very good analysis of the standard can be found below.
>>>
>>> http://stackoverflow.com/questions/3523145/pointer-arithmetic-for-void-pointer-in-c
>>
>> The analysis would seem to show that arithmetic may be acceptable, but
>> it doesn't say that void pointers must be treated like char pointers.
>> In my view, this would make sense:
>>
>> char *cptr;
>> void *vptr;
>>
>> Since
>> cptr++;
>> is equivalent to
>> cptr = (char *)((uintptr_t)cptr + sizeof(*cptr));
>>
>> therefore
>>
>> vptr++;
>> should be equivalent to
>> vptr = (void *)((uintptr_t)vptr + sizeof(*vptr));
>> That is, vptr++ should be equivalent to vptr += 0 because sizeof(void)
>> should be 0 if allowed.
>
> sizeof(void) == 1
>
> With GCC at least.

It's not valid C (0 is just how I think it should be if allowed). Also
GCC can reject it even with std=gnu89 (default, C89 with GNU
extensions):
$ cat void.c
unsigned long x = sizeof(void);
$ gcc -pedantic void.c -c
void.c:1: warning: invalid application of 'sizeof' to a void type

> Regards,
>
> Anthony Liguori
>
>>>
>>> Regards,
>>>
>>> Anthony Liguori
>>>
>>>>
>>>>>
>>>>> Is there a work in progress to build GCC with visual studio?
>>>>> If yes what are the chances KVM device assignment
>>>>> will work on windows?
>>>>
>>>> IIRC there was really a project to use KVM on Windows and another
>>>> project to build QEMU with MSVC.
>>>>
>>>>>
>>>>> Look QEMU codebase is what it is. Unless you rework all existing
>>>>> code to confirm to your taste, I do not see why you NACK valid new code
>>>>> unless it confirms to same.
>>>>
>>>> Yes, I'd be happy to fix the style with huge patches at once. But our
>>>> fearless leader does not agree, so we are stuck with the codebase
>>>> being what it is until it is fixed one step at a time.
>>>>
>>>>>
>>>>>> >
>>>>>> > /mjt
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe kvm" in
>>>> the body of a message to address@hidden
>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html