* [PATCH] scsi: bfa: Avoid implicit enum conversion in bfad_im_post_vendor_event
@ 2018-09-26 4:54 Nathan Chancellor
2018-09-27 23:35 ` Nick Desaulniers
2018-09-27 23:56 ` [PATCH v2] " Nathan Chancellor
0 siblings, 2 replies; 6+ messages in thread
From: Nathan Chancellor @ 2018-09-26 4:54 UTC (permalink / raw)
To: Anil Gurumurthy, Sudarsana Kalluru, James E.J. Bottomley,
Martin K. Petersen
Cc: linux-scsi, linux-kernel, Nick Desaulniers, Nathan Chancellor
Clang warns when one enumerated type is implicitly converted to another.
drivers/scsi/bfa/bfa_fcs_lport.c:379:26: warning: implicit conversion
from enumeration type 'enum bfa_lport_aen_event' to different
enumeration type 'enum bfa_ioc_aen_event' [-Wenum-conversion]
BFA_AEN_CAT_LPORT, event);
^~~~~
The root cause of these warnings is the bfad_im_post_vendor_event
function, which expects a value from enum bfa_ioc_aen_event but there
are multiple instances of values from enums bfa_port_aen_event,
bfa_audit_aen_event, and bfa_lport_aen_event being used in this
function.
Given that this doesn't appear to be a problem since cat helps with
differentiating the events, just change evt's type to int so that no
conversion needs to happen and Clang won't warn.
Link: https://github.com/ClangBuiltLinux/linux/issues/147
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---
The alternate way of fixing these warnings is to explicitly cast the
conversion when calling the function but since there are about 8-10 of
these warnings, it seems logical to just change the function definiton
which is cleaner in my opinion.
See commits 3eb95feac113 ("mm/zsmalloc.c: change stat type parameter to
int") and 04fecbf51b3c ("mm: memcontrol: use int for event/state
parameter in several functions") for similar fixes.
drivers/scsi/bfa/bfad_im.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/bfa/bfad_im.h b/drivers/scsi/bfa/bfad_im.h
index e61ed8dad0b4..bd4ac187fd8e 100644
--- a/drivers/scsi/bfa/bfad_im.h
+++ b/drivers/scsi/bfa/bfad_im.h
@@ -143,7 +143,7 @@ struct bfad_im_s {
static inline void bfad_im_post_vendor_event(struct bfa_aen_entry_s *entry,
struct bfad_s *drv, int cnt,
enum bfa_aen_category cat,
- enum bfa_ioc_aen_event evt)
+ int evt)
{
struct timespec64 ts;
--
2.19.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] scsi: bfa: Avoid implicit enum conversion in bfad_im_post_vendor_event
2018-09-26 4:54 [PATCH] scsi: bfa: Avoid implicit enum conversion in bfad_im_post_vendor_event Nathan Chancellor
@ 2018-09-27 23:35 ` Nick Desaulniers
2018-09-27 23:49 ` Nathan Chancellor
2018-09-27 23:56 ` [PATCH v2] " Nathan Chancellor
1 sibling, 1 reply; 6+ messages in thread
From: Nick Desaulniers @ 2018-09-27 23:35 UTC (permalink / raw)
To: Nathan Chancellor
Cc: anil.gurumurthy, sudarsana.kalluru, James E.J. Bottomley,
Martin K. Petersen, linux-scsi, LKML
On Tue, Sep 25, 2018 at 9:58 PM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> Clang warns when one enumerated type is implicitly converted to another.
>
> drivers/scsi/bfa/bfa_fcs_lport.c:379:26: warning: implicit conversion
> from enumeration type 'enum bfa_lport_aen_event' to different
> enumeration type 'enum bfa_ioc_aen_event' [-Wenum-conversion]
> BFA_AEN_CAT_LPORT, event);
> ^~~~~
>
> The root cause of these warnings is the bfad_im_post_vendor_event
> function, which expects a value from enum bfa_ioc_aen_event but there
> are multiple instances of values from enums bfa_port_aen_event,
> bfa_audit_aen_event, and bfa_lport_aen_event being used in this
> function.
Indeed, it seems that bfad_im_post_vendor_event() assigns this parameter to
161 entry->aen_type = evt;
which is defined as:
1456 u32 aen_type;
so already we know that aen_type is meant to be a grab bag of enum
values. bfad_im_post_vendor_event() is already passed many different
types of enums, as you mention. Does changing aen_type to an `int`
produce further warnings, because it would be nice to have that change
in this one, too. AFAICT, it's only ever saved away in a containing
struct.
>
> Given that this doesn't appear to be a problem since cat helps with
> differentiating the events, just change evt's type to int so that no
> conversion needs to happen and Clang won't warn.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/147
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
>
> The alternate way of fixing these warnings is to explicitly cast the
> conversion when calling the function but since there are about 8-10 of
> these warnings, it seems logical to just change the function definiton
> which is cleaner in my opinion.
>
> See commits 3eb95feac113 ("mm/zsmalloc.c: change stat type parameter to
> int") and 04fecbf51b3c ("mm: memcontrol: use int for event/state
> parameter in several functions") for similar fixes.
>
> drivers/scsi/bfa/bfad_im.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/bfa/bfad_im.h b/drivers/scsi/bfa/bfad_im.h
> index e61ed8dad0b4..bd4ac187fd8e 100644
> --- a/drivers/scsi/bfa/bfad_im.h
> +++ b/drivers/scsi/bfa/bfad_im.h
> @@ -143,7 +143,7 @@ struct bfad_im_s {
> static inline void bfad_im_post_vendor_event(struct bfa_aen_entry_s *entry,
> struct bfad_s *drv, int cnt,
> enum bfa_aen_category cat,
> - enum bfa_ioc_aen_event evt)
> + int evt)
> {
> struct timespec64 ts;
>
> --
> 2.19.0
>
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] scsi: bfa: Avoid implicit enum conversion in bfad_im_post_vendor_event
2018-09-27 23:35 ` Nick Desaulniers
@ 2018-09-27 23:49 ` Nathan Chancellor
0 siblings, 0 replies; 6+ messages in thread
From: Nathan Chancellor @ 2018-09-27 23:49 UTC (permalink / raw)
To: Nick Desaulniers
Cc: anil.gurumurthy, sudarsana.kalluru, James E.J. Bottomley,
Martin K. Petersen, linux-scsi, LKML
On Thu, Sep 27, 2018 at 04:35:37PM -0700, Nick Desaulniers wrote:
> On Tue, Sep 25, 2018 at 9:58 PM Nathan Chancellor
> <natechancellor@gmail.com> wrote:
> >
> > Clang warns when one enumerated type is implicitly converted to another.
> >
> > drivers/scsi/bfa/bfa_fcs_lport.c:379:26: warning: implicit conversion
> > from enumeration type 'enum bfa_lport_aen_event' to different
> > enumeration type 'enum bfa_ioc_aen_event' [-Wenum-conversion]
> > BFA_AEN_CAT_LPORT, event);
> > ^~~~~
> >
> > The root cause of these warnings is the bfad_im_post_vendor_event
> > function, which expects a value from enum bfa_ioc_aen_event but there
> > are multiple instances of values from enums bfa_port_aen_event,
> > bfa_audit_aen_event, and bfa_lport_aen_event being used in this
> > function.
>
> Indeed, it seems that bfad_im_post_vendor_event() assigns this parameter to
> 161 entry->aen_type = evt;
>
> which is defined as:
>
> 1456 u32 aen_type;
>
> so already we know that aen_type is meant to be a grab bag of enum
> values. bfad_im_post_vendor_event() is already passed many different
> types of enums, as you mention. Does changing aen_type to an `int`
> produce further warnings, because it would be nice to have that change
> in this one, too. AFAICT, it's only ever saved away in a containing
> struct.
>
No, it doesn't introduce any new warnings. I can send that as a v2 here shortly.
Nathan
> >
> > Given that this doesn't appear to be a problem since cat helps with
> > differentiating the events, just change evt's type to int so that no
> > conversion needs to happen and Clang won't warn.
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/147
> > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> > ---
> >
> > The alternate way of fixing these warnings is to explicitly cast the
> > conversion when calling the function but since there are about 8-10 of
> > these warnings, it seems logical to just change the function definiton
> > which is cleaner in my opinion.
> >
> > See commits 3eb95feac113 ("mm/zsmalloc.c: change stat type parameter to
> > int") and 04fecbf51b3c ("mm: memcontrol: use int for event/state
> > parameter in several functions") for similar fixes.
> >
> > drivers/scsi/bfa/bfad_im.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/scsi/bfa/bfad_im.h b/drivers/scsi/bfa/bfad_im.h
> > index e61ed8dad0b4..bd4ac187fd8e 100644
> > --- a/drivers/scsi/bfa/bfad_im.h
> > +++ b/drivers/scsi/bfa/bfad_im.h
> > @@ -143,7 +143,7 @@ struct bfad_im_s {
> > static inline void bfad_im_post_vendor_event(struct bfa_aen_entry_s *entry,
> > struct bfad_s *drv, int cnt,
> > enum bfa_aen_category cat,
> > - enum bfa_ioc_aen_event evt)
> > + int evt)
> > {
> > struct timespec64 ts;
> >
> > --
> > 2.19.0
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] scsi: bfa: Avoid implicit enum conversion in bfad_im_post_vendor_event
2018-09-26 4:54 [PATCH] scsi: bfa: Avoid implicit enum conversion in bfad_im_post_vendor_event Nathan Chancellor
2018-09-27 23:35 ` Nick Desaulniers
@ 2018-09-27 23:56 ` Nathan Chancellor
2018-10-02 23:06 ` Nick Desaulniers
2018-10-17 1:45 ` Martin K. Petersen
1 sibling, 2 replies; 6+ messages in thread
From: Nathan Chancellor @ 2018-09-27 23:56 UTC (permalink / raw)
To: Anil Gurumurthy, Sudarsana Kalluru, James E.J. Bottomley,
Martin K. Petersen
Cc: linux-scsi, linux-kernel, Nick Desaulniers, Nathan Chancellor
Clang warns when one enumerated type is implicitly converted to another.
drivers/scsi/bfa/bfa_fcs_lport.c:379:26: warning: implicit conversion
from enumeration type 'enum bfa_lport_aen_event' to different
enumeration type 'enum bfa_ioc_aen_event' [-Wenum-conversion]
BFA_AEN_CAT_LPORT, event);
^~~~~
The root cause of these warnings is the bfad_im_post_vendor_event
function, which expects a value from enum bfa_ioc_aen_event but there
are multiple instances of values from enums bfa_port_aen_event,
bfa_audit_aen_event, and bfa_lport_aen_event being used in this
function.
Given that this doesn't appear to be a problem since cat helps with
differentiating the events, just change evt's type to int so that no
conversion needs to happen and Clang won't warn. Update aen_type's type
in bfa_aen_entry_s as members that hold enumerated types should be int.
Link: https://github.com/ClangBuiltLinux/linux/issues/147
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---
v1 -> v2:
* Update aen_type's type in bfa_aen_entry_s to match evt
drivers/scsi/bfa/bfa_defs_svc.h | 2 +-
drivers/scsi/bfa/bfad_im.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/bfa/bfa_defs_svc.h b/drivers/scsi/bfa/bfa_defs_svc.h
index 3d0c96a5c873..c19c26e0e405 100644
--- a/drivers/scsi/bfa/bfa_defs_svc.h
+++ b/drivers/scsi/bfa/bfa_defs_svc.h
@@ -1453,7 +1453,7 @@ union bfa_aen_data_u {
struct bfa_aen_entry_s {
struct list_head qe;
enum bfa_aen_category aen_category;
- u32 aen_type;
+ int aen_type;
union bfa_aen_data_u aen_data;
u64 aen_tv_sec;
u64 aen_tv_usec;
diff --git a/drivers/scsi/bfa/bfad_im.h b/drivers/scsi/bfa/bfad_im.h
index e61ed8dad0b4..bd4ac187fd8e 100644
--- a/drivers/scsi/bfa/bfad_im.h
+++ b/drivers/scsi/bfa/bfad_im.h
@@ -143,7 +143,7 @@ struct bfad_im_s {
static inline void bfad_im_post_vendor_event(struct bfa_aen_entry_s *entry,
struct bfad_s *drv, int cnt,
enum bfa_aen_category cat,
- enum bfa_ioc_aen_event evt)
+ int evt)
{
struct timespec64 ts;
--
2.19.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2] scsi: bfa: Avoid implicit enum conversion in bfad_im_post_vendor_event
2018-09-27 23:56 ` [PATCH v2] " Nathan Chancellor
@ 2018-10-02 23:06 ` Nick Desaulniers
2018-10-17 1:45 ` Martin K. Petersen
1 sibling, 0 replies; 6+ messages in thread
From: Nick Desaulniers @ 2018-10-02 23:06 UTC (permalink / raw)
To: Nathan Chancellor
Cc: anil.gurumurthy, sudarsana.kalluru, James E.J. Bottomley,
Martin K. Petersen, linux-scsi, LKML
On Thu, Sep 27, 2018 at 4:57 PM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> Clang warns when one enumerated type is implicitly converted to another.
>
> drivers/scsi/bfa/bfa_fcs_lport.c:379:26: warning: implicit conversion
> from enumeration type 'enum bfa_lport_aen_event' to different
> enumeration type 'enum bfa_ioc_aen_event' [-Wenum-conversion]
> BFA_AEN_CAT_LPORT, event);
> ^~~~~
>
> The root cause of these warnings is the bfad_im_post_vendor_event
> function, which expects a value from enum bfa_ioc_aen_event but there
> are multiple instances of values from enums bfa_port_aen_event,
> bfa_audit_aen_event, and bfa_lport_aen_event being used in this
> function.
>
> Given that this doesn't appear to be a problem since cat helps with
> differentiating the events, just change evt's type to int so that no
> conversion needs to happen and Clang won't warn. Update aen_type's type
> in bfa_aen_entry_s as members that hold enumerated types should be int.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/147
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
>
> v1 -> v2:
>
> * Update aen_type's type in bfa_aen_entry_s to match evt
Thanks for respinning with this addition.
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
>
> drivers/scsi/bfa/bfa_defs_svc.h | 2 +-
> drivers/scsi/bfa/bfad_im.h | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/bfa/bfa_defs_svc.h b/drivers/scsi/bfa/bfa_defs_svc.h
> index 3d0c96a5c873..c19c26e0e405 100644
> --- a/drivers/scsi/bfa/bfa_defs_svc.h
> +++ b/drivers/scsi/bfa/bfa_defs_svc.h
> @@ -1453,7 +1453,7 @@ union bfa_aen_data_u {
> struct bfa_aen_entry_s {
> struct list_head qe;
> enum bfa_aen_category aen_category;
> - u32 aen_type;
> + int aen_type;
> union bfa_aen_data_u aen_data;
> u64 aen_tv_sec;
> u64 aen_tv_usec;
> diff --git a/drivers/scsi/bfa/bfad_im.h b/drivers/scsi/bfa/bfad_im.h
> index e61ed8dad0b4..bd4ac187fd8e 100644
> --- a/drivers/scsi/bfa/bfad_im.h
> +++ b/drivers/scsi/bfa/bfad_im.h
> @@ -143,7 +143,7 @@ struct bfad_im_s {
> static inline void bfad_im_post_vendor_event(struct bfa_aen_entry_s *entry,
> struct bfad_s *drv, int cnt,
> enum bfa_aen_category cat,
> - enum bfa_ioc_aen_event evt)
> + int evt)
> {
> struct timespec64 ts;
>
> --
> 2.19.0
>
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] scsi: bfa: Avoid implicit enum conversion in bfad_im_post_vendor_event
2018-09-27 23:56 ` [PATCH v2] " Nathan Chancellor
2018-10-02 23:06 ` Nick Desaulniers
@ 2018-10-17 1:45 ` Martin K. Petersen
1 sibling, 0 replies; 6+ messages in thread
From: Martin K. Petersen @ 2018-10-17 1:45 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Anil Gurumurthy, Sudarsana Kalluru, James E.J. Bottomley,
Martin K. Petersen, linux-scsi, linux-kernel, Nick Desaulniers
Nathan,
> Clang warns when one enumerated type is implicitly converted to another.
>
> drivers/scsi/bfa/bfa_fcs_lport.c:379:26: warning: implicit conversion
> from enumeration type 'enum bfa_lport_aen_event' to different
> enumeration type 'enum bfa_ioc_aen_event' [-Wenum-conversion]
> BFA_AEN_CAT_LPORT, event);
> ^~~~~
Applied to 4.20/scsi-queue.
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-10-17 1:46 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-26 4:54 [PATCH] scsi: bfa: Avoid implicit enum conversion in bfad_im_post_vendor_event Nathan Chancellor
2018-09-27 23:35 ` Nick Desaulniers
2018-09-27 23:49 ` Nathan Chancellor
2018-09-27 23:56 ` [PATCH v2] " Nathan Chancellor
2018-10-02 23:06 ` Nick Desaulniers
2018-10-17 1:45 ` Martin K. Petersen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).