diff mbox

[ndctl] libndctl, nfit: Fix in/out sizes for error injection commands

Message ID CAPcyv4iMU1KUb5_S+B52b6bZXcisNmdB2PY6t8x7ANxQveMNjA@mail.gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Dan Williams Nov. 8, 2017, 6:48 p.m. UTC
On Wed, Nov 8, 2017 at 10:07 AM, Dan Williams <dan.j.williams@intel.com> wrote:
> On Tue, Nov 7, 2017 at 2:50 PM, Vishal Verma <vishal.l.verma@intel.com> wrote:
>> The input/output size bounds being set in the various nd_bus_cmd_new_*
>> helpers for error injection commands were larger than they needed to be,
>> and platforms could reject these. Fix the bounds to be exactly as the
>> spec describes.
>>
>> Cc: Dan Williams <dan.j.williams@intel.com>
>> Reported-by: Dariusz Dokupil <dariusz.dokupil@intel.com>
>> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
>> ---
>>  ndctl/lib/nfit.c | 16 ++++++++--------
>>  1 file changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/ndctl/lib/nfit.c b/ndctl/lib/nfit.c
>> index fb6af32..6346fd9 100644
>> --- a/ndctl/lib/nfit.c
>> +++ b/ndctl/lib/nfit.c
>> @@ -164,9 +164,9 @@ struct ndctl_cmd *ndctl_bus_cmd_new_err_inj(struct ndctl_bus *bus)
>>         cmd->status = 1;
>>         pkg = (struct nd_cmd_pkg *)&cmd->cmd_buf[0];
>>         pkg->nd_command = NFIT_CMD_ARS_INJECT_SET;
>> -       pkg->nd_size_in = (2 * sizeof(u64)) + sizeof(u32);
>> -       pkg->nd_size_out = cmd_length;
>> -       pkg->nd_fw_size = cmd_length;
>> +       pkg->nd_size_in = (2 * sizeof(u64)) + sizeof(u8);
>
> How about:
>
>     pkg->nd_size_in = offset_of(struct nd_cmd_ars_err_inj, status);

Actually, something like this to make the pkg-> settings and
inter-relations clearer:

Comments

Verma, Vishal L Nov. 8, 2017, 7:09 p.m. UTC | #1
On Wed, 2017-11-08 at 10:48 -0800, Dan Williams wrote:
> On Wed, Nov 8, 2017 at 10:07 AM, Dan Williams <dan.j.williams@intel.c
> om> wrote:
> > On Tue, Nov 7, 2017 at 2:50 PM, Vishal Verma <vishal.l.verma@intel.
> > com> wrote:
> > > The input/output size bounds being set in the various
> > > nd_bus_cmd_new_*
> > > helpers for error injection commands were larger than they needed
> > > to be,
> > > and platforms could reject these. Fix the bounds to be exactly as
> > > the
> > > spec describes.
> > > 
> > > Cc: Dan Williams <dan.j.williams@intel.com>
> > > Reported-by: Dariusz Dokupil <dariusz.dokupil@intel.com>
> > > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> > > ---
> > >  ndctl/lib/nfit.c | 16 ++++++++--------
> > >  1 file changed, 8 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/ndctl/lib/nfit.c b/ndctl/lib/nfit.c
> > > index fb6af32..6346fd9 100644
> > > --- a/ndctl/lib/nfit.c
> > > +++ b/ndctl/lib/nfit.c
> > > @@ -164,9 +164,9 @@ struct ndctl_cmd
> > > *ndctl_bus_cmd_new_err_inj(struct ndctl_bus *bus)
> > >         cmd->status = 1;
> > >         pkg = (struct nd_cmd_pkg *)&cmd->cmd_buf[0];
> > >         pkg->nd_command = NFIT_CMD_ARS_INJECT_SET;
> > > -       pkg->nd_size_in = (2 * sizeof(u64)) + sizeof(u32);
> > > -       pkg->nd_size_out = cmd_length;
> > > -       pkg->nd_fw_size = cmd_length;
> > > +       pkg->nd_size_in = (2 * sizeof(u64)) + sizeof(u8);
> > 
> > How about:
> > 
> >     pkg->nd_size_in = offset_of(struct nd_cmd_ars_err_inj, status);
> 
> Actually, something like this to make the pkg-> settings and
> inter-relations clearer:
> 
> diff --git a/ndctl/lib/nfit.c b/ndctl/lib/nfit.c
> index 6346fd90f0e2..21c567ad5d4a 100644
> --- a/ndctl/lib/nfit.c
> +++ b/ndctl/lib/nfit.c
> @@ -164,9 +164,9 @@ struct ndctl_cmd
> *ndctl_bus_cmd_new_err_inj(struct
> ndctl_bus *bus)
>         cmd->status = 1;
>         pkg = (struct nd_cmd_pkg *)&cmd->cmd_buf[0];
>         pkg->nd_command = NFIT_CMD_ARS_INJECT_SET;
> -       pkg->nd_size_in = (2 * sizeof(u64)) + sizeof(u8);
> -       pkg->nd_size_out = sizeof(u32);
> -       pkg->nd_fw_size = sizeof(u32);
> +       pkg->nd_size_in = offsetof(struct nd_cmd_ars_err_inj,
> status);
> +       pkg->nd_size_out = sizeof(struct nd_cmd_ars_err_inj) - pkg-
> >nd_size_in;
> +       pkg->nd_fw_size = pkg->nd_size_out;
>         err_inj = (struct nd_cmd_ars_err_inj *)&pkg->nd_payload[0];
>         cmd->firmware_status = &err_inj->status;

Yep, I like that. I will resend.
diff mbox

Patch

diff --git a/ndctl/lib/nfit.c b/ndctl/lib/nfit.c
index 6346fd90f0e2..21c567ad5d4a 100644
--- a/ndctl/lib/nfit.c
+++ b/ndctl/lib/nfit.c
@@ -164,9 +164,9 @@  struct ndctl_cmd *ndctl_bus_cmd_new_err_inj(struct
ndctl_bus *bus)
        cmd->status = 1;
        pkg = (struct nd_cmd_pkg *)&cmd->cmd_buf[0];
        pkg->nd_command = NFIT_CMD_ARS_INJECT_SET;
-       pkg->nd_size_in = (2 * sizeof(u64)) + sizeof(u8);
-       pkg->nd_size_out = sizeof(u32);
-       pkg->nd_fw_size = sizeof(u32);
+       pkg->nd_size_in = offsetof(struct nd_cmd_ars_err_inj, status);
+       pkg->nd_size_out = sizeof(struct nd_cmd_ars_err_inj) - pkg->nd_size_in;
+       pkg->nd_fw_size = pkg->nd_size_out;
        err_inj = (struct nd_cmd_ars_err_inj *)&pkg->nd_payload[0];
        cmd->firmware_status = &err_inj->status;