[dm-devel] [PATCH] multipathd: avoid unnecessary path read-only reloads

[Martin Wilck]

On Tue, 2021-12-14 at 18:49 -0600, Benjamin Marzinski wrote:
> On Fri, Dec 03, 2021 at 08:25:00AM +0000, Martin Wilck wrote:
> > On Thu, 2021-11-18 at 16:47 -0600, Benjamin Marzinski wrote:
> > > +int
> > > +sysfs_get_ro (struct path *pp)
> > > +{
> > > +       int ro;
> > > +       char buff[3]; /* Either "0\n\0" or "1\n\0" */
> > > +
> > > +       if (!pp->udev)
> > > +               return -1;
> > > +
> > > +       if (sysfs_attr_get_value(pp->udev, "ro", buff,
> > > sizeof(buff))
> > > <= 0) {
> > > +               condlog(3, "%s: Cannot read ro attribute in
> > > sysfs",
> > > pp->dev);
> > > +               return -1;
> > > +       }
> > > +
> > > +       if (sscanf(buff, "%d\n", &ro) != 1 || ro < 0 || ro > 1) {
> > 
> > This is ok, but for self-evidently correct code in multipath-tools,
> > it'd be better to read just 2 bytes and set buff[2] = '\0'
> > explicitly.
> > I haven't checked, but coverity might stumble on this.
> 
> Actually this is just the way sysfs_attr_get_value() works. You have
> to
> provide a larger buffer than you will read (in this case, ro will
> only
> ever have two bytes), otherwise it will think that it overflowed.
> sysfs_attr_get_value() also does the NULL termination itself.
> 

Well, fine then. It's a nit anyway.

Martin