svn commit: r368187 - head/sys/dev/nvme

[John Baldwin]

On 11/30/20 9:04 AM, Warner Losh wrote:
> On Mon, Nov 30, 2020 at 9:56 AM Michal Meloun <meloun.michal at gmail.com>
> wrote:
> 
>>
>>
>> On 30.11.2020 17:02, Ian Lepore wrote:
>>> On Mon, 2020-11-30 at 14:51 +0000, Michal Meloun wrote:
>>>> Author: mmel
>>>> Date: Mon Nov 30 14:51:48 2020
>>>> New Revision: 368187
>>>> URL: https://svnweb.freebsd.org/changeset/base/368187
>>>>
>>>> Log:
>>>>    Unbreak r368167 in userland. Decorate unused arguments.
>>>>
>>>>    Reported by:      kp, tuexen, jenkins, and many others
>>>>    MFC with: r368167
>>>>
>>>> Modified:
>>>>    head/sys/dev/nvme/nvme.h
>>>>
>>>> Modified: head/sys/dev/nvme/nvme.h
>>>> =====================================================================
>>>> =========
>>>> --- head/sys/dev/nvme/nvme.h Mon Nov 30 14:49:13 2020        (r368186)
>>>> +++ head/sys/dev/nvme/nvme.h Mon Nov 30 14:51:48 2020        (r368187)
>>>> @@ -1728,9 +1728,15 @@ extern int nvme_use_nvd;
>>>>
>>>>   #endif /* _KERNEL */
>>>>
>>>> +#if _BYTE_ORDER != _LITTLE_ENDIAN
>>>> +#define MODIF
>>>> +#else
>>>> +#define MODIF __unused
>>>> +#endif
>>>> +
>>>>   /* Endianess conversion functions for NVMe structs */
>>>>   static inline
>>>> -void        nvme_completion_swapbytes(struct nvme_completion *s)
>>>> +void        nvme_completion_swapbytes(struct nvme_completion *s MODIF)
>>>
>>> IMO, this is pretty ugly, it causes the brain to screech to a halt when
>>> you see it.  Why not just add an unconditional __unused to the
>>> functions?  The unused attribute is defined as marking the variable as
>>> "potentially unused" -- there is no penalty for having it there and
>>> then actually using the variable.
>>>
>>
>> I understand, (and I have significant tendency to agree) but I did not
>> find more correct way how to do it.
>> Are you sure that __unused is defined as *potentially* unused?  I cannot
>> find nothing about this and you known how are compiler guys creative
>> with generating of new warnings...
>> I known that C++17 have 'maybe_unused' attribute, but relationship to
>> standard '__unused' looks unclear.
>>
>> In any case, I have not single problem to change this to the proposed
>> style if we found that this is the optimal way.
>>
> 
> __unused means 'don't warn me if this is unused' elsewhere in the tree.
> Better to use it here.

Alternatively, given you already are using #ifdef's in all the function
bodies, you could instead do something like this:

#if _BYTE_ORDER != _LITTLE_ENDIAN

/* Existing functions without #if */

#else
#define nvme_completion_swapbytes(s)

/* Empty macros for the rest */

#endif

This gives only a single #if instead of duplicating them in
each function, and it avoids the need for __unused.

-- 
John Baldwin