[SLOF] [PATCH slof 07/13] e1000: Compile with -Wextra

[Alexey Kardashevskiy]

On 29/01/2021 17:38, Thomas Huth wrote:
> On 29/01/2021 03.01, Alexey Kardashevskiy wrote:
>>
>>
>> On 29/01/2021 02:04, Thomas Huth wrote:
>>> On 27/01/2021 09.57, Alexey Kardashevskiy wrote:
>>>> -Wextra enables a bunch of rather useful checks which this fixes.
>>>>
>>>> Signed-off-by: Alexey Kardashevskiy <aik at ozlabs.ru>
>>>> ---
>>>>   lib/libe1k/e1k.c | 16 ++++++++--------
>>>>   1 file changed, 8 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/lib/libe1k/e1k.c b/lib/libe1k/e1k.c
>>>> index 4dd7d2eb97c2..3bbc75fba199 100644
>>>> --- a/lib/libe1k/e1k.c
>>>> +++ b/lib/libe1k/e1k.c
>>>> @@ -163,7 +163,7 @@ static const e1k_dev_t e1k_dev[] = {
>>>>       { 0x1016, E1K_82540, "82540EP Mobile" },
>>>>       { 0x1017, E1K_82540, "82540EP Desktop" },
>>>>       { 0x100E, E1K_82540, "82540EM Desktop" },
>>>> -    { 0     , 0 }
>>>> +    { 0 }
>>>>   };
>>>
>>> Ack for the above change (or you could simply add a NULL as third 
>>> parameter), but are the changes below really required?
>>
>>
>> These was a warning that the third parameter is not initialized. So it 
>> could be either one zero or three zeroes. I find one more future proof.
>>
>>
>>> Seem like you only tried to get rid of the unused-parameter warnings 
>>> which you finally switched off anyway?
>>
>> No, it is a different warning. Thanks,
> 
> So which warnings do you get that you tried to address with the changes 
> below? I only get one warning here with -Wextra -Wno-unused-parameter, 
> and that's the one that you fixed with above hunk.


The initial warning was about unused parameter. Which I decided to use, 
hence this new MIN() call down there which in turn triggered 
signed/unsigned comparison so I changed the type. Makes sense?


> 
>   Thomas
> 
> 
>>>
>>>>   /*
>>>> @@ -182,8 +182,8 @@ check_driver(uint16_t vendor_id, uint16_t 
>>>> device_id);
>>>>   static int e1k_init(net_driver_t *driver);
>>>>   static int e1k_term(void);
>>>> -static int e1k_xmit(char *f_buffer_pc, int f_len_i);
>>>> -static int e1k_receive(char *f_buffer_pc, int f_len_i);
>>>> +static int e1k_xmit(char *f_buffer_pc, unsigned f_len_i);
>>>> +static int e1k_receive(char *f_buffer_pc, unsigned f_len_i);
>>>>   /**
>>>>    * Translate virtual to "physical" address, ie. an address
>>>> @@ -549,11 +549,11 @@ e1k_mac_init(uint8_t *f_mac_pu08)
>>>>    * e1k_receive
>>>>    */
>>>>   static int
>>>> -e1k_receive(char *f_buffer_pc, int f_len_i)
>>>> +e1k_receive(char *f_buffer_pc, unsigned f_len_i)
>>>>   {
>>>>       uint32_t    l_rdh_u32 = e1k_rd32(RDH);    // this includes 
>>>> needed dummy read
>>>>       e1k_rx_desc_st    *rx;
>>>> -    int        l_ret_i;
>>>> +    unsigned    l_ret_i;
>>>>       #ifdef E1K_DEBUG
>>>>       #ifdef E1K_SHOW_RCV_DATA
>>>> @@ -589,11 +589,11 @@ e1k_receive(char *f_buffer_pc, int f_len_i)
>>>>        * copy the data
>>>>        */
>>>>       memcpy((uint8_t *) f_buffer_pc, 
>>>> dma2virt(bswap_64(rx->m_buffer_u64)),
>>>> -        (size_t) l_ret_i);
>>>> +        (size_t) MIN(l_ret_i, f_len_i));
>>>>       #ifdef E1K_DEBUG
>>>>       #if defined(E1K_SHOW_RCV) || defined(E1K_SHOW_RCV_DATA)
>>>> -    printf("e1k: %d bytes received\n", l_ret_i);
>>>> +    printf("e1k: %d bytes received (max %d)\n", l_ret_i, f_len_i);
>>>>       #endif
>>>>       #ifdef E1K_SHOW_RCV_DATA
>>>> @@ -631,7 +631,7 @@ e1k_receive(char *f_buffer_pc, int f_len_i)
>>>>   }
>>>>   static int
>>>> -e1k_xmit(char *f_buffer_pc, int f_len_i)
>>>> +e1k_xmit(char *f_buffer_pc, unsigned f_len_i)
>>>>   {
>>>>       uint32_t    l_tdh_u32 = e1k_rd32(TDH);
>>>>       uint32_t    l_tdt_u32 = e1k_rd32(TDT);
>>>>
>>>
>>
> 

-- 
Alexey