Re: svn commit: r368187 - head/sys/dev/nvme
On Mon, 2020-11-30 at 17:56 +0100, Michal Meloun wrote: > > On 30.11.2020 17:02, Ian Lepore wrote: > > On Mon, 2020-11-30 at 14:51 +, 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. > > Michal The __unused macro is defined in cdefs.h under #if __GNUC_PREREQ__(2, 7) as attribute((unused)) and the gcc docs for that attribute say "This attribute, attached to a [variable|function], means that the variable is meant to be possibly unused. GCC will not produce a warning for this variable." -- Ian ___ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r368187 - head/sys/dev/nvme
On 11/30/20 9:04 AM, Warner Losh wrote: > On Mon, Nov 30, 2020 at 9:56 AM Michal Meloun > wrote: > >> >> >> On 30.11.2020 17:02, Ian Lepore wrote: >>> On Mon, 2020-11-30 at 14:51 +, 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 -voidnvme_completion_swapbytes(struct nvme_completion *s) +voidnvme_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 ___ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r368187 - head/sys/dev/nvme
On Mon, Nov 30, 2020 at 9:56 AM Michal Meloun wrote: > > > On 30.11.2020 17:02, Ian Lepore wrote: > > On Mon, 2020-11-30 at 14:51 +, 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 > >> -voidnvme_completion_swapbytes(struct nvme_completion *s) > >> +voidnvme_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. Warner ___ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r368187 - head/sys/dev/nvme
On 30.11.2020 17:02, Ian Lepore wrote: On Mon, 2020-11-30 at 14:51 +, 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.hMon Nov 30 14:49:13 2020(r368186) +++ head/sys/dev/nvme/nvme.hMon 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. Michal ___ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r368187 - head/sys/dev/nvme
On Mon, 2020-11-30 at 14:51 +, 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. -- Ian ___ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
