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