| Message ID | 20180911052014.2206-1-hegdevasant@linux.vnet.ibm.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | hdata: Make sure FW feature name is not NULL | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | master/apply_patch Successfully applied |
| snowpatch_ozlabs/make_check | success | Test make_check on branch master |
Hi Vasant, Bikeshedding a bit but a minor review comment: "hdata: Make sure FW feature name is not NULL" I think it should be "not Empty" rather than "not NULL" since you are checking for length and 'name' and 'feature->name' being a static arrays wont be NULL. Vasant Hegde <hegdevasant@linux.vnet.ibm.com> writes: > CC: Oliver O'Halloran <oohall@gmail.com> > Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> > --- > hdata/spira.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/hdata/spira.c b/hdata/spira.c > index c820c4daf..3a26f17ae 100644 > --- a/hdata/spira.c > +++ b/hdata/spira.c > @@ -1213,6 +1213,11 @@ static void add_iplparams_features(const struct HDIF_common_hdr *iplp) > name[sizeof(name)-1] = '\0'; > flags = be64_to_cpu(feature->flags); > > + if (strlen(name) == 0) { > + prlog(PR_DEBUG, "IPLPARAMS: FW feature name is NULL\n"); > + continue; > + } > +
Vaibhav Jain <vaibhav@linux.vnet.ibm.com> writes: > Hi Vasant, > > Bikeshedding a bit but a minor review comment: > > "hdata: Make sure FW feature name is not NULL" > > I think it should be "not Empty" rather than "not NULL" since you are > checking for length and 'name' and 'feature->name' being a static arrays > wont be NULL. Fixed the bikeshed when merging. Merged to master as of 9d9395c3d542b5b2949b6ba3d5c0a8f1618b77e0
On 09/13/2018 02:22 PM, Stewart Smith wrote: > Vaibhav Jain <vaibhav@linux.vnet.ibm.com> writes: >> Hi Vasant, >> >> Bikeshedding a bit but a minor review comment: >> >> "hdata: Make sure FW feature name is not NULL" >> >> I think it should be "not Empty" rather than "not NULL" since you are >> checking for length and 'name' and 'feature->name' being a static arrays >> wont be NULL. > > Fixed the bikeshed when merging. Thanks! -Vasant
diff --git a/hdata/spira.c b/hdata/spira.c index c820c4daf..3a26f17ae 100644 --- a/hdata/spira.c +++ b/hdata/spira.c @@ -1213,6 +1213,11 @@ static void add_iplparams_features(const struct HDIF_common_hdr *iplp) name[sizeof(name)-1] = '\0'; flags = be64_to_cpu(feature->flags); + if (strlen(name) == 0) { + prlog(PR_DEBUG, "IPLPARAMS: FW feature name is NULL\n"); + continue; + } + prlog(PR_DEBUG, "IPLPARAMS: FW feature %s = %016"PRIx64"\n", name, flags);
CC: Oliver O'Halloran <oohall@gmail.com> Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> --- hdata/spira.c | 5 +++++ 1 file changed, 5 insertions(+)