On 27.10.20 20:09, Andrea Bastoni wrote:
> On 27/10/2020 14:36, Jan Kiszka wrote:
>> On 22.10.20 19:58, Andrea Bastoni wrote:
>>> Signed-off-by: Andrea Bastoni <[email protected]>
>>> ---
>>>  hypervisor/control.c                | 2 ++
>>>  hypervisor/include/jailhouse/unit.h | 2 +-
>>>  hypervisor/paging.c                 | 4 ++++
>>>  hypervisor/pci.c                    | 6 ++++++
>>>  hypervisor/printk.c                 | 2 +-
>>>  5 files changed, 14 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/hypervisor/control.c b/hypervisor/control.c
>>> index 0078ef19..81b7614f 100644
>>> --- a/hypervisor/control.c
>>> +++ b/hypervisor/control.c
>>> @@ -884,6 +884,8 @@ static int hypervisor_disable(struct per_cpu *cpu_data)
>>>  
>>>  static long hypervisor_get_info(struct per_cpu *cpu_data, unsigned long 
>>> type)
>>>  {
>>> +   (void)cpu_data;
>>> +
>>>     switch (type) {
>>>     case JAILHOUSE_INFO_MEM_POOL_SIZE:
>>>             return mem_pool.pages;
>>> diff --git a/hypervisor/include/jailhouse/unit.h 
>>> b/hypervisor/include/jailhouse/unit.h
>>> index 40e1cbfe..39dfc056 100644
>>> --- a/hypervisor/include/jailhouse/unit.h
>>> +++ b/hypervisor/include/jailhouse/unit.h
>>> @@ -38,7 +38,7 @@ struct unit {
>>>     static void __name##_shutdown(void) { }
>>>  
>>>  #define DEFINE_UNIT_MMIO_COUNT_REGIONS_STUB(__name)                        
>>> \
>>> -   static unsigned int __name##_mmio_count_regions(struct cell *cell) \
>>> +   static unsigned int __name##_mmio_count_regions(struct cell *cell 
>>> __attribute__((unused))) \
>>
>> Overlong.
>>
>>>     { return 0; }
>>>  
>>>  extern struct unit __unit_array_start[0], __unit_array_end[0];
>>> diff --git a/hypervisor/paging.c b/hypervisor/paging.c
>>> index 75d5da59..0573cfd2 100644
>>> --- a/hypervisor/paging.c
>>> +++ b/hypervisor/paging.c
>>> @@ -55,6 +55,8 @@ struct paging_structures parking_pt;
>>>   */
>>>  unsigned long paging_get_phys_invalid(pt_entry_t pte, unsigned long virt)
>>>  {
>>> +   (void)pte;
>>> +   (void)virt;
>>>     return INVALID_PHYS_ADDR;
>>>  }
>>>  
>>> @@ -507,6 +509,8 @@ void *paging_map_device(unsigned long phys, unsigned 
>>> long size)
>>>   */
>>>  void paging_unmap_device(unsigned long phys, void *virt, unsigned long 
>>> size)
>>>  {
>>> +   (void)phys;
>>> +
>>>     /* Cannot fail if paired with paging_map_device. */
>>>     paging_destroy(&hv_paging_structs, (unsigned long)virt, size,
>>>                    PAGING_NON_COHERENT);
>>> diff --git a/hypervisor/pci.c b/hypervisor/pci.c
>>> index fe85ae2f..79b6f920 100644
>>> --- a/hypervisor/pci.c
>>> +++ b/hypervisor/pci.c
>>> @@ -205,6 +205,8 @@ pci_find_capability(struct pci_device *device, u16 
>>> address)
>>>  enum pci_access pci_cfg_read_moderate(struct pci_device *device, u16 
>>> address,
>>>                                   unsigned int size, u32 *value)
>>>  {
>>> +   (void)size;
>>> +
>>>     const struct jailhouse_pci_capability *cap;
>>>     unsigned int bar_no, cap_offs;
>>>  
>>> @@ -253,6 +255,8 @@ enum pci_access pci_cfg_read_moderate(struct pci_device 
>>> *device, u16 address,
>>>  static int pci_update_msix(struct pci_device *device,
>>>                        const struct jailhouse_pci_capability *cap)
>>>  {
>>> +   (void)cap;
>>> +
>>>     unsigned int n;
>>>     int result;
>>>  
>>> @@ -404,6 +408,8 @@ invalid_access:
>>>  static enum mmio_result pci_mmconfig_access_handler(void *arg,
>>>                                                 struct mmio_access *mmio)
>>>  {
>>> +   (void)arg;
>>> +
>>>     u32 reg_addr = mmio->address & 0xfff;
>>>     u16 bdf = mmio->address >> 12;
>>>     struct pci_device *device;
>>> diff --git a/hypervisor/printk.c b/hypervisor/printk.c
>>> index a32ff8c4..6f149f0f 100644
>>> --- a/hypervisor/printk.c
>>> +++ b/hypervisor/printk.c
>>> @@ -46,7 +46,7 @@ static void console_write(const char *msg)
>>>     console.busy = false;
>>>  }
>>>  
>>> -static void dbg_write_stub(const char *msg)
>>> +static void dbg_write_stub(const char *msg __attribute__((unused)))
>>>  {
>>>  }
>>>  
>>>
>>
>> First, the "solution" looks a bit inconsistent (unused vs. (void)arg).
>> Second, I'm not yet sure there is a lot of value in this. Can you come
>> up with relevant issues that this can reveal?
> 
> I've thought as well if adding Wno-unused-parameter would be better here.
> 
> During development it can help refactoring and to avoid leaving unneeded
> functions around.

How does that help with unneeded functions?

> 
> It probably helps the compiler to avoid spilling for extern functions, but I
> have not checked.
> 
> Since it's included in Wextra I simply didn't want to suppress it by default.
> 

The kernel does that as well, so it seems a reasonable choice.
Specifically when you have implementations of the same interface by
different archs or units, there can easily be valid cases of unused
parameters. Annotating them all will become very verbose with no clear
value IMHO.

Jan

-- 
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux

-- 
You received this message because you are subscribed to the Google Groups 
"Jailhouse" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To view this discussion on the web visit 
https://groups.google.com/d/msgid/jailhouse-dev/133be7d2-65d5-d90c-a4c0-418c31760c52%40siemens.com.

Reply via email to