> +static ssize_t cbd_myhost_show(struct device *dev, > + struct device_attribute *attr, > + char *buf) > +{ > + struct cbd_transport *cbdt; > + struct cbd_host *host; > + > + cbdt = container_of(dev, struct cbd_transport, device); > + > + host = cbdt->host; > + if (!host) > + return 0; > + > + return sprintf(buf, "%d\n", host->host_id); snprintf() ? > +} > + > +static DEVICE_ATTR(my_host_id, 0400, cbd_myhost_show, NULL); > + > +enum { [...] > + > +static ssize_t cbd_adm_store(struct device *dev, > + struct device_attribute *attr, > + const char *ubuf, > + size_t size) > +{ > + int ret; > + char *buf; > + struct cbd_adm_options opts = { 0 }; > + struct cbd_transport *cbdt; > + reverse tree order that matches rest of your code ? > + if (!capable(CAP_SYS_ADMIN)) > + return -EPERM; > + > + cbdt = container_of(dev, struct cbd_transport, device); > + > + buf = kmemdup(ubuf, size + 1, GFP_KERNEL); > + if (IS_ERR(buf)) { > + pr_err("failed to dup buf for adm option: %d", (int)PTR_ERR(buf)); > + return PTR_ERR(buf); > + } > + buf[size] = '\0'; > + ret = parse_adm_options(cbdt, buf, &opts); > + if (ret < 0) { > + kfree(buf); > + return ret; > + } > + kfree(buf); > + standard format is using goto out and having only on kfree() at the end of the function ... > + switch (opts.op) { > + case CBDT_ADM_OP_B_START: > + break; > + case CBDT_ADM_OP_B_STOP: > + break; > + case CBDT_ADM_OP_B_CLEAR: > + break; > + case CBDT_ADM_OP_DEV_START: > + break; > + case CBDT_ADM_OP_DEV_STOP: > + break; > + default: > + pr_err("invalid op: %d\n", opts.op); > + return -EINVAL; > + } > + > + if (ret < 0) > + return ret; > + > + return size; > +} > + [...] > +static struct cbd_transport *cbdt_alloc(void) > +{ > + struct cbd_transport *cbdt; > + int ret; > + > + cbdt = kzalloc(sizeof(struct cbd_transport), GFP_KERNEL); > + if (!cbdt) { > + return NULL; > + } no braces needed for single statements in if ... applies rest of the code ... -ck