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. -- Alastair D'Silva Open Source Developer Linux Technology Centre, IBM Australia mob: 0423 762 819