| Message ID | 147630484183.28641.3160414367749359017.stgit@dwillia2-desk3.amr.corp.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Oct 12, 2016 at 1:40 PM, Dan Williams <dan.j.williams@intel.com> wrote: > Fix warnings of the form: > > libndctl-hpe1.c: In function 'hpe1_smart_valid': > libndctl-hpe1.c:78:15: warning: dereferencing type-punned pointer will > break strict-aliasing rules [-Wstrict-aliasing] > CMD_HPE1(cmd)->gen.nd_family != NVDIMM_FAMILY_HPE1 || > ^ > libndctl-hpe1.c:79:15: warning: dereferencing type-punned pointer will > break strict-aliasing rules [-Wstrict-aliasing] > CMD_HPE1(cmd)->gen.nd_command != NDN_HPE1_CMD_SMART || > ^ > libndctl-hpe1.c: In function 'hpe1_cmd_smart_get_flags': > libndctl-hpe1.c:93:55: warning: dereferencing type-punned pointer will > break strict-aliasing rules [-Wstrict-aliasing] > libndctl-hpe1.c:93:55: warning: dereferencing type-punned pointer will > break strict-aliasing rules [-Wstrict-aliasing] > libndctl-hpe1.c: In function 'hpe1_cmd_smart_get_health': > libndctl-hpe1.c:121:56: warning: dereferencing type-punned pointer will > break strict-aliasing rules [-Wstrict-aliasing] > libndctl-hpe1.c:121:56: warning: dereferencing type-punned pointer will > break strict-aliasing rules [-Wstrict-aliasing] > > ...for distributions that include "-Wstrict-aliasing" in CFLAGS. > > Cc: Brian Boylston <brian.boylston@hpe.com> > Signed-off-by: Dan Williams <dan.j.williams@intel.com> Brian, it occurs to me that I do not have unit test coverage for hpe1 command functionality. Can you confirm that this patch does not break anything? For extra credit, I wouldn't say no if you instrumented the kernel's nfit_test infrastructure to provide sample hpe1 data payload results.
Dan Williams wrote on 2016-10-13: > On Wed, Oct 12, 2016 at 1:40 PM, Dan Williams <dan.j.williams@intel.com> wrote: >> Fix warnings of the form: >> >> libndctl-hpe1.c: In function 'hpe1_smart_valid': >> libndctl-hpe1.c:78:15: warning: dereferencing type-punned pointer will >> break strict-aliasing rules [-Wstrict-aliasing] >> CMD_HPE1(cmd)->gen.nd_family != NVDIMM_FAMILY_HPE1 || >> ^ >> libndctl-hpe1.c:79:15: warning: dereferencing type-punned pointer will >> break strict-aliasing rules [-Wstrict-aliasing] >> CMD_HPE1(cmd)->gen.nd_command != NDN_HPE1_CMD_SMART || >> ^ >> libndctl-hpe1.c: In function 'hpe1_cmd_smart_get_flags': >> libndctl-hpe1.c:93:55: warning: dereferencing type-punned pointer will >> break strict-aliasing rules [-Wstrict-aliasing] >> libndctl-hpe1.c:93:55: warning: dereferencing type-punned pointer will >> break strict-aliasing rules [-Wstrict-aliasing] >> libndctl-hpe1.c: In function 'hpe1_cmd_smart_get_health': >> libndctl-hpe1.c:121:56: warning: dereferencing type-punned pointer will >> break strict-aliasing rules [-Wstrict-aliasing] >> libndctl-hpe1.c:121:56: warning: dereferencing type-punned pointer will >> break strict-aliasing rules [-Wstrict-aliasing] >> ...for distributions that include "-Wstrict-aliasing" in CFLAGS. >> >> Cc: Brian Boylston <brian.boylston@hpe.com> >> Signed-off-by: Dan Williams <dan.j.williams@intel.com> > > Brian, it occurs to me that I do not have unit test coverage for hpe1 > command functionality. Can you confirm that this patch does not break > anything? Yes! I actually tested this earlier today using the pending branch, and it seems to work fine: # ndctl --version 54.44.g4a4c09e # ndctl list -D -i -H [ { "dev":"nmem1", "id":"802c-0f-1612-122f8182", "state":"disabled", "health":{ "health_state":"fatal", "temperature_celsius":25.000000, "spares_percentage":99, "alarm_temperature":false, "alarm_spares":false, "temperature_threshold":50.000000, "spares_threshold":20, "life_used_percentage":29, "shutdown_state":"dirty" } }, { "dev":"nmem3", "id":"802c-0f-1612-122f81da", "state":"disabled", "health":{ "health_state":"ok", "temperature_celsius":25.000000, "spares_percentage":99, "alarm_temperature":false, "alarm_spares":false, "temperature_threshold":50.000000, "spares_threshold":20, "life_used_percentage":29, "shutdown_state":"clean" } }, ... nmem1 here has an injected error. > > For extra credit, I wouldn't say no if you instrumented the kernel's > nfit_test infrastructure to provide sample hpe1 data payload results. Sure, when I get a chance, I'll take a look. Thanks! Brian
diff --git a/ndctl/lib/libndctl-hpe1.c b/ndctl/lib/libndctl-hpe1.c index 48d8e02cf46e..ec542527e509 100644 --- a/ndctl/lib/libndctl-hpe1.c +++ b/ndctl/lib/libndctl-hpe1.c @@ -19,11 +19,9 @@ #include "ndctl-hpe1.h" -#define CMD_HPE1(_c) ((struct ndn_pkg_hpe1 *)((_c)->cmd_buf)) -#define CMD_HPE1_SMART(_c) \ - ((struct ndn_hpe1_smart_data *)(CMD_HPE1(_c)->u.smart.data)) -#define CMD_HPE1_SMART_THRESH(_c) \ - ((struct ndn_hpe1_smart_threshold_data *)(CMD_HPE1(_c)->u.thresh.data)) +#define CMD_HPE1(_c) ((_c)->hpe1) +#define CMD_HPE1_SMART(_c) (CMD_HPE1(_c)->u.smart.data) +#define CMD_HPE1_SMART_THRESH(_c) (CMD_HPE1(_c)->u.thresh.data) static struct ndctl_cmd *hpe1_dimm_cmd_new_smart(struct ndctl_dimm *dimm) { diff --git a/ndctl/lib/libndctl-private.h b/ndctl/lib/libndctl-private.h index 4363f2373d7b..0dc9e30cdb6e 100644 --- a/ndctl/lib/libndctl-private.h +++ b/ndctl/lib/libndctl-private.h @@ -31,6 +31,7 @@ #include <ndctl/libndctl.h> #include <ccan/endian/endian.h> #include <ccan/short_types/short_types.h> +#include "ndctl-hpe1.h" #define SZ_16M 0x01000000 @@ -191,6 +192,7 @@ struct ndctl_cmd { #ifdef HAVE_NDCTL_CLEAR_ERROR struct nd_cmd_clear_error clear_err[0]; #endif + struct ndn_pkg_hpe1 hpe1[0]; struct nd_cmd_smart smart[0]; struct nd_cmd_smart_threshold smart_t[0]; struct nd_cmd_get_config_size get_size[0]; diff --git a/ndctl/lib/ndctl-hpe1.h b/ndctl/lib/ndctl-hpe1.h index 0d41aab4e64f..b050831ec2c4 100644 --- a/ndctl/lib/ndctl-hpe1.h +++ b/ndctl/lib/ndctl-hpe1.h @@ -33,12 +33,6 @@ enum { }; /* NDN_HPE1_CMD_SMART */ -struct ndn_hpe1_smart { - __u32 in_valid_flags; - __u32 status; - __u8 data[124]; -} __attribute__((packed)); - /* ndn_hpe1_smart.in_valid_flags / ndn_hpe1_smart_data.out_valid_flags */ #define NDN_HPE1_SMART_HEALTH_VALID (1 << 0) #define NDN_HPE1_SMART_TEMP_VALID (1 << 1) @@ -112,12 +106,16 @@ struct ndn_hpe1_smart_data { __u8 vnd_spec_data[60]; } __attribute__((packed)); -/* NDN_HPE1_CMD_SMART_THRESHOLD */ -struct ndn_hpe1_smart_threshold { +struct ndn_hpe1_smart { + __u32 in_valid_flags; __u32 status; - __u8 data[32]; + union { + __u8 buf[124]; + struct ndn_hpe1_smart_data data[0]; + }; } __attribute__((packed)); +/* NDN_HPE1_CMD_SMART_THRESHOLD */ struct ndn_hpe1_smart_threshold_data { __u16 threshold_alarm_ctl; __u16 temp_threshold; @@ -134,6 +132,14 @@ struct ndn_hpe1_smart_threshold_data { __u64 res4; } __attribute__((packed)); +struct ndn_hpe1_smart_threshold { + __u32 status; + union { + __u8 buf[32]; + struct ndn_hpe1_smart_threshold_data data[0]; + }; +} __attribute__((packed)); + /* NDN_HPE1_CMD_GET_CONFIG_SIZE */ struct ndn_hpe1_get_config_size { __u32 status;
Fix warnings of the form: libndctl-hpe1.c: In function 'hpe1_smart_valid': libndctl-hpe1.c:78:15: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing] CMD_HPE1(cmd)->gen.nd_family != NVDIMM_FAMILY_HPE1 || ^ libndctl-hpe1.c:79:15: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing] CMD_HPE1(cmd)->gen.nd_command != NDN_HPE1_CMD_SMART || ^ libndctl-hpe1.c: In function 'hpe1_cmd_smart_get_flags': libndctl-hpe1.c:93:55: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing] libndctl-hpe1.c:93:55: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing] libndctl-hpe1.c: In function 'hpe1_cmd_smart_get_health': libndctl-hpe1.c:121:56: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing] libndctl-hpe1.c:121:56: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing] ...for distributions that include "-Wstrict-aliasing" in CFLAGS. Cc: Brian Boylston <brian.boylston@hpe.com> Signed-off-by: Dan Williams <dan.j.williams@intel.com> --- ndctl/lib/libndctl-hpe1.c | 8 +++----- ndctl/lib/libndctl-private.h | 2 ++ ndctl/lib/ndctl-hpe1.h | 24 +++++++++++++++--------- 3 files changed, 20 insertions(+), 14 deletions(-)