diff mbox

[PULL,V2,00/20] Net patches

Message ID 02F257B6-4170-4503-BF4C-270D831740B1@daynix.com (mailing list archive)
State New, archived
Headers show

Commit Message

Dmitry Fleytman May 25, 2016, 8:31 a.m. UTC 
> On 25 May 2016, at 11:04 AM, Jason Wang <jasowang@redhat.com> wrote:
> 
> 
> 
> On 2016年05月24日 21:41, Peter Maydell wrote:
>> On 24 May 2016 at 08:49, Jason Wang <jasowang@redhat.com> wrote:
>>> The following changes since commit 3a2da52a99582ac2c0e29e548bb1fc28e2cdb84a:
>>> 
>>>   net/tap: Allocating Large sized arrays to heap (2016-05-24 15:30:41 +0800)
>>> 
>>> are available in the git repository at:
>>> 
>>>   https://github.com/jasowang/qemu.git tags/net-pull-request
>>> 
>>> for you to fetch changes up to f99d30aaf53e5b501dd3405007232db963ba65c6:
>>> 
>>>   net/net: Add SocketReadState for reuse codes (2016-05-24 15:30:42 +0800)
>>> 
>>> ----------------------------------------------------------------
>>> 
>>> Main changes:
>>> - e1000e emulation
>>> - convet vmxnet3 to use DMA api
>>> Changes from V1:
>>> - fix 32bit build
>> Hi; I'm afraid this doesn't build on clang, which dislikes duplicate
>> typedefs:
>> 
>> In file included from /home/petmay01/linaro/qemu-for-merges/hw/net/e1000e.c:47:
>> /home/petmay01/linaro/qemu-for-merges/hw/net/e1000e_core.h:110:3:
>> error: redefinition of typedef 'E1000ECore' is a C11 feature
>> [-Werror,-Wtypedef-redefinition]
>> } E1000ECore;
>>   ^
>> /home/petmay01/linaro/qemu-for-merges/hw/net/e1000e_core.h:43:29:
>> note: previous definition is here
>> typedef struct E1000Core_st E1000ECore;
>>                             ^
>> 
>> thanks
>> -- PMM
>> 
> 
> Have a try on my machine, but looks like it works for me (./configure --cc=clang).  version is
> 
> clang version 3.8.0-2ubuntu3 (tags/RELEASE_380/final)
> Target: x86_64-pc-linux-gnu
> Thread model: posix
> InstalledDir: /usr/bin
> Found candidate GCC installation: /usr/bin/../lib/gcc/x86_64-linux-gnu/5.3.1
> Found candidate GCC installation: /usr/bin/../lib/gcc/x86_64-linux-gnu/6.0.0
> Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/5.3.1
> Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/6.0.0
> Selected GCC installation: /usr/bin/../lib/gcc/x86_64-linux-gnu/5.3.1
> Candidate multilib: .;@m64
> Selected multilib: .;@m64
> 
> Looks like "-Werror,-Wtypedef-redefinition" was not used by default. After I add them to extra_cflags, e1000e can still build successfully.
> 
> Is there anything I missed?

Hi Jason,

My clang 3.7.0 compiles this code fine as well. I just tried an older clang (3.4.2) and it produces build error as reported by Peter.

The problematic patch is “net: Introduce e1000e device emulation"

Fix is simple:

Thanks,
Dmitry

> 
> Thanks
> 
>

Comments

Jason Wang May 25, 2016, 11:01 a.m. UTC | #1 
On 2016年05月25日 16:31, Dmitry Fleytman wrote:
>> On 25 May 2016, at 11:04 AM, Jason Wang <jasowang@redhat.com> wrote:
>>
>>
>>
>> On 2016年05月24日 21:41, Peter Maydell wrote:
>>> On 24 May 2016 at 08:49, Jason Wang <jasowang@redhat.com> wrote:
>>>> The following changes since commit 3a2da52a99582ac2c0e29e548bb1fc28e2cdb84a:
>>>>
>>>>    net/tap: Allocating Large sized arrays to heap (2016-05-24 15:30:41 +0800)
>>>>
>>>> are available in the git repository at:
>>>>
>>>>    https://github.com/jasowang/qemu.git tags/net-pull-request
>>>>
>>>> for you to fetch changes up to f99d30aaf53e5b501dd3405007232db963ba65c6:
>>>>
>>>>    net/net: Add SocketReadState for reuse codes (2016-05-24 15:30:42 +0800)
>>>>
>>>> ----------------------------------------------------------------
>>>>
>>>> Main changes:
>>>> - e1000e emulation
>>>> - convet vmxnet3 to use DMA api
>>>> Changes from V1:
>>>> - fix 32bit build
>>> Hi; I'm afraid this doesn't build on clang, which dislikes duplicate
>>> typedefs:
>>>
>>> In file included from /home/petmay01/linaro/qemu-for-merges/hw/net/e1000e.c:47:
>>> /home/petmay01/linaro/qemu-for-merges/hw/net/e1000e_core.h:110:3:
>>> error: redefinition of typedef 'E1000ECore' is a C11 feature
>>> [-Werror,-Wtypedef-redefinition]
>>> } E1000ECore;
>>>    ^
>>> /home/petmay01/linaro/qemu-for-merges/hw/net/e1000e_core.h:43:29:
>>> note: previous definition is here
>>> typedef struct E1000Core_st E1000ECore;
>>>                              ^
>>>
>>> thanks
>>> -- PMM
>>>
>> Have a try on my machine, but looks like it works for me (./configure --cc=clang).  version is
>>
>> clang version 3.8.0-2ubuntu3 (tags/RELEASE_380/final)
>> Target: x86_64-pc-linux-gnu
>> Thread model: posix
>> InstalledDir: /usr/bin
>> Found candidate GCC installation: /usr/bin/../lib/gcc/x86_64-linux-gnu/5.3.1
>> Found candidate GCC installation: /usr/bin/../lib/gcc/x86_64-linux-gnu/6.0.0
>> Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/5.3.1
>> Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/6.0.0
>> Selected GCC installation: /usr/bin/../lib/gcc/x86_64-linux-gnu/5.3.1
>> Candidate multilib: .;@m64
>> Selected multilib: .;@m64
>>
>> Looks like "-Werror,-Wtypedef-redefinition" was not used by default. After I add them to extra_cflags, e1000e can still build successfully.
>>
>> Is there anything I missed?
> Hi Jason,
>
> My clang 3.7.0 compiles this code fine as well. I just tried an older clang (3.4.2) and it produces build error as reported by Peter.

Aha, I see.

>
> The problematic patch is “net: Introduce e1000e device emulation"
>
> Fix is simple:
>
> diff --git a/hw/net/e1000e_core.h b/hw/net/e1000e_core.h
> index ff0ffbe..7a885e4 100644
> --- a/hw/net/e1000e_core.h
> +++ b/hw/net/e1000e_core.h
> @@ -55,7 +55,7 @@ typedef struct E1000IntrDelayTimer_st {
>       E1000ECore *core;
>   } E1000IntrDelayTimer;
>
> -typedef struct E1000Core_st {
> +struct E1000Core_st {
>       uint32_t mac[E1000E_MAC_SIZE];
>       uint16_t phy[E1000E_PHY_PAGES][E1000E_PHY_PAGE_SIZE];
>       uint16_t eeprom[E1000E_EEPROM_SIZE];
> @@ -107,7 +107,7 @@ typedef struct E1000Core_st {
>       NICState *owner_nic;
>       PCIDevice *owner;
>       void (*owner_start_recv)(PCIDevice *d);
> -} E1000ECore;
> +};
>
> Would you amend it yourself or should we send updated series?

I will amend it myself.

Thanks

>
> Thanks,
> Dmitry
>
>> Thanks
>>
>>
>
Dmitry Fleytman May 25, 2016, 11:04 a.m. UTC | #2 
> On 25 May 2016, at 14:01 PM, Jason Wang <jasowang@redhat.com> wrote:
> 
> 
> 
> On 2016年05月25日 16:31, Dmitry Fleytman wrote:
>>> On 25 May 2016, at 11:04 AM, Jason Wang <jasowang@redhat.com> wrote:
>>> 
>>> 
>>> 
>>> On 2016年05月24日 21:41, Peter Maydell wrote:
>>>> On 24 May 2016 at 08:49, Jason Wang <jasowang@redhat.com> wrote:
>>>>> The following changes since commit 3a2da52a99582ac2c0e29e548bb1fc28e2cdb84a:
>>>>> 
>>>>>   net/tap: Allocating Large sized arrays to heap (2016-05-24 15:30:41 +0800)
>>>>> 
>>>>> are available in the git repository at:
>>>>> 
>>>>>   https://github.com/jasowang/qemu.git tags/net-pull-request
>>>>> 
>>>>> for you to fetch changes up to f99d30aaf53e5b501dd3405007232db963ba65c6:
>>>>> 
>>>>>   net/net: Add SocketReadState for reuse codes (2016-05-24 15:30:42 +0800)
>>>>> 
>>>>> ----------------------------------------------------------------
>>>>> 
>>>>> Main changes:
>>>>> - e1000e emulation
>>>>> - convet vmxnet3 to use DMA api
>>>>> Changes from V1:
>>>>> - fix 32bit build
>>>> Hi; I'm afraid this doesn't build on clang, which dislikes duplicate
>>>> typedefs:
>>>> 
>>>> In file included from /home/petmay01/linaro/qemu-for-merges/hw/net/e1000e.c:47:
>>>> /home/petmay01/linaro/qemu-for-merges/hw/net/e1000e_core.h:110:3:
>>>> error: redefinition of typedef 'E1000ECore' is a C11 feature
>>>> [-Werror,-Wtypedef-redefinition]
>>>> } E1000ECore;
>>>>   ^
>>>> /home/petmay01/linaro/qemu-for-merges/hw/net/e1000e_core.h:43:29:
>>>> note: previous definition is here
>>>> typedef struct E1000Core_st E1000ECore;
>>>>                             ^
>>>> 
>>>> thanks
>>>> -- PMM
>>>> 
>>> Have a try on my machine, but looks like it works for me (./configure --cc=clang).  version is
>>> 
>>> clang version 3.8.0-2ubuntu3 (tags/RELEASE_380/final)
>>> Target: x86_64-pc-linux-gnu
>>> Thread model: posix
>>> InstalledDir: /usr/bin
>>> Found candidate GCC installation: /usr/bin/../lib/gcc/x86_64-linux-gnu/5.3.1
>>> Found candidate GCC installation: /usr/bin/../lib/gcc/x86_64-linux-gnu/6.0.0
>>> Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/5.3.1
>>> Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/6.0.0
>>> Selected GCC installation: /usr/bin/../lib/gcc/x86_64-linux-gnu/5.3.1
>>> Candidate multilib: .;@m64
>>> Selected multilib: .;@m64
>>> 
>>> Looks like "-Werror,-Wtypedef-redefinition" was not used by default. After I add them to extra_cflags, e1000e can still build successfully.
>>> 
>>> Is there anything I missed?
>> Hi Jason,
>> 
>> My clang 3.7.0 compiles this code fine as well. I just tried an older clang (3.4.2) and it produces build error as reported by Peter.
> 
> Aha, I see.
> 
>> 
>> The problematic patch is “net: Introduce e1000e device emulation"
>> 
>> Fix is simple:
>> 
>> diff --git a/hw/net/e1000e_core.h b/hw/net/e1000e_core.h
>> index ff0ffbe..7a885e4 100644
>> --- a/hw/net/e1000e_core.h
>> +++ b/hw/net/e1000e_core.h
>> @@ -55,7 +55,7 @@ typedef struct E1000IntrDelayTimer_st {
>>      E1000ECore *core;
>>  } E1000IntrDelayTimer;
>> 
>> -typedef struct E1000Core_st {
>> +struct E1000Core_st {
>>      uint32_t mac[E1000E_MAC_SIZE];
>>      uint16_t phy[E1000E_PHY_PAGES][E1000E_PHY_PAGE_SIZE];
>>      uint16_t eeprom[E1000E_EEPROM_SIZE];
>> @@ -107,7 +107,7 @@ typedef struct E1000Core_st {
>>      NICState *owner_nic;
>>      PCIDevice *owner;
>>      void (*owner_start_recv)(PCIDevice *d);
>> -} E1000ECore;
>> +};
>> 
>> Would you amend it yourself or should we send updated series?
> 
> I will amend it myself.

Thanks, Jason!

> 
> Thanks
> 
>> 
>> Thanks,
>> Dmitry
>> 
>>> Thanks
diff mbox

Patch

diff --git a/hw/net/e1000e_core.h b/hw/net/e1000e_core.h
index ff0ffbe..7a885e4 100644
--- a/hw/net/e1000e_core.h
+++ b/hw/net/e1000e_core.h
@@ -55,7 +55,7 @@  typedef struct E1000IntrDelayTimer_st {
     E1000ECore *core;
 } E1000IntrDelayTimer;

-typedef struct E1000Core_st {
+struct E1000Core_st {
     uint32_t mac[E1000E_MAC_SIZE];
     uint16_t phy[E1000E_PHY_PAGES][E1000E_PHY_PAGE_SIZE];
     uint16_t eeprom[E1000E_EEPROM_SIZE];
@@ -107,7 +107,7 @@  typedef struct E1000Core_st {
     NICState *owner_nic;
     PCIDevice *owner;
     void (*owner_start_recv)(PCIDevice *d);
-} E1000ECore;
+};

Would you amend it yourself or should we send updated series?