On Fri, Jan 3, 2020 at 1:03 AM Bart Van Assche <bvanassche@xxxxxxx> wrote:
>
> On 12/30/19 2:29 AM, Jack Wang wrote:
> > +static const match_table_t rnbd_opt_tokens = {
> > + { RNBD_OPT_PATH, "path=%s" },
> > + { RNBD_OPT_DEV_PATH, "device_path=%s" },
> > + { RNBD_OPT_ACCESS_MODE, "access_mode=%s" },
> > + { RNBD_OPT_SESSNAME, "sessname=%s" },
> > + { RNBD_OPT_ERR, NULL },
> > +};
>
>
> Please follow the example of other kernel code and change
> "{<tab>...<tab>}" into "{ ... }".
ok.
>
> > +/* remove new line from string */
> > +static void strip(char *s)
> > +{
> > + char *p = s;
> > +
> > + while (*s != '\0') {
> > + if (*s != '\n')
> > + *p++ = *s++;
> > + else
> > + ++s;
> > + }
> > + *p = '\0';
> > +}
>
> Does this function change a multiline string into a single line? I'm not
> sure that is how sysfs input should be processed ... Is this perhaps
> what you want?
>
> static inline void kill_final_newline(char *str)
> {
> char *newline = strrchr(str, '\n');
>
> if (newline && !newline[1])
> *newline = 0;
probably you meant "*newline = '\0'"
Your version only removes the last newline, our version removes all
the newlines in the string.
> }
>
> > +static struct kobj_attribute rnbd_clt_map_device_attr =
> > + __ATTR(map_device, 0644,
> > + rnbd_clt_map_device_show, rnbd_clt_map_device_store);
>
> Could __ATTR_RW() have been used here?
As replied in other emails, it can be used, but we prefer to keep the
prefix part.
>
> Thanks,
>
> Bart.
Thanks Bart
