Re: [PATCH v6 18/25] rnbd: client: sysfs interface functions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux