On Mon, 2020-03-02 at 10:42 +1100, Alastair D'Silva wrote: > On Fri, 2020-02-28 at 08:15 +0100, Greg Kroah-Hartman wrote: > > On Fri, Feb 28, 2020 at 05:25:31PM +1100, Andrew Donnellan wrote: > > > On 21/2/20 2:27 pm, Alastair D'Silva wrote: > > > > +int ocxlpmem_sysfs_add(struct ocxlpmem *ocxlpmem) > > > > +{ > > > > + int i, rc; > > > > + > > > > + for (i = 0; i < ARRAY_SIZE(attrs); i++) { > > > > + rc = device_create_file(&ocxlpmem->dev, > > > > &attrs[i]); > > > > + if (rc) { > > > > + for (; --i >= 0;) > > > > + device_remove_file(&ocxlpmem- > > > > >dev, > > > > &attrs[i]); > > > > > > I'd rather avoid weird for loop constructs if possible. > > > > > > Is it actually dangerous to call device_remove_file() on an attr > > > that hasn't > > > been added? If not then I'd rather define an err: label and loop > > > over the > > > whole array there. > > > > None of this should be used at all, just use attribute groups > > properly > > and the driver core will handle this all for you. > > > > device_create/remove_file should never be called by anyone anymore > > if > > at all > > possible. > > > > thanks, > > > > greg k-h > > Thanks, I'll rework it to use the .groups member of struct > pci_driver. > I ended up making these available as DIMM attributes instead. -- Alastair D'Silva Open Source Developer Linux Technology Centre, IBM Australia mob: 0423 762 819