lkml.org 
[lkml]   [2015]   [May]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 3/6] edac, mce_amd_inj: Modify flags attrigute to use string arguments
    On 5/29/2015 8:49 AM, Borislav Petkov wrote:
    > On Wed, May 27, 2015 at 02:03:35PM -0500, Aravind Gopalakrishnan wrote:
    >>
    >> +#define MAX_FLAG_OPT_SIZE 10
    > Why 10?

    No specific reason. Just an arbitrary max value that we won't hit right
    now or in the future.

    > This should be 2 and increased when another, longer injection type
    > string gets introduced.

    Okay, I'll make it 2 here and will increase it to 3 in the next patch
    when I introduce injection types for the interrupts.

    >
    >>
    >> - *val = m->inject_flags;
    >> + for (i = 0; i <= N_INJ_TYPES; i++) {
    >> + inj_op = flags_options[i];
    >> + if (inj_op && strncmp(inj_op, buf, cnt) == 0)
    >> + break;
    >> + }
    > What's wrong with:
    >
    > for (i = 0; i < N_INJ_TYPES; i++) {
    > if (!strncmp(flags_options[i], buf, strlen(flags_options[i]))) {
    > inj_type = i;
    > return 0;
    > }
    > }
    >
    > return -EINVAL;

    Hmm. That should work. Will simplify it in the next version.
    I think I had a NULL in flags_options[] to denote the invalid option.
    With your simplification, that can be removed. I'll make that change too.

    And the 'cnt' value that I used won't be necessary. So, will remove that
    from __set_inj() definition as well.

    >> +static ssize_t flags_write(struct file *filp, const char __user *ubuf,
    >> + size_t cnt, loff_t *ppos)
    >> +{
    >> + char buf[MAX_FLAG_OPT_SIZE + 1];
    >> + int err;
    >> + size_t ret;
    >> +
    >> + ret = cnt;
    > You're assigning cnt to ret here...
    >
    >> +
    >> + if (cnt > MAX_FLAG_OPT_SIZE)
    >> + cnt = MAX_FLAG_OPT_SIZE;
    > ... but correcting cnt afterwards. The assignment should be *after* that
    > correction.

    With the change to MAX_FLAG_OPT_SIZE to 2 (or 3 depending on the max
    size), this check is
    really an error check.

    Shall I make err = -EINVAL by default and return that here if the
    condition is not satisfied?

    Thanks,
    -Aravind.


    \
     
     \ /
      Last update: 2015-05-29 20:41    [W:3.567 / U:0.060 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site