Messages in this thread | | | Date | Fri, 19 Jun 2020 16:02:06 +0200 | From | Greg Kroah-Hartman <> | Subject | Re: [PATCH] Ability to read the MKTME status from userspace |
| |
On Fri, Jun 19, 2020 at 10:51:32AM -0300, Daniel Gutson wrote: > > > +enum mktme_status_type get_mktme_status(void) > > > +{ > > > + return mktme_status; > > > +} > > > +EXPORT_SYMBOL_GPL(get_mktme_status); > > > > prefix of the subsystem first please: > > mktme_get_status > > > > OK. > > > Or, better yet, why not just export the variable directly? Why is this > > a function at all? > > Because I want this to be read only.
read-only to who?
> > > + > > > +/* Buffer to return: always 3 because of the following chars: > > > + * value \n \0 > > > + */ > > > +#define BUFFER_SIZE 3 > > > > Why a define? > > Do you suggest `static const int` instead?
Why do you need it at all?
> > > + > > > + sprintf(tmp, "%d\n", (int)get_mktme_status() & 1); > > > + return simple_read_from_buffer(buf, count, ppos, tmp, sizeof(tmp)); > > > +} > > > + > > > +static const struct file_operations mktme_status_ops = { > > > + .read = mktme_status_read, > > > +}; > > > + > > > +static int __init mod_init(void) > > > +{ > > > + mktme_dir = securityfs_create_dir("mktme", NULL); > > > + if (IS_ERR(mktme_dir)) { > > > + pr_err("Couldn't create mktme sysfs dir\n"); > > > + return -1; > > > > Don't make up random error numbers, use the EWHATEVER defines please. > > > > Could you please suggest one?
Why not return the error given to you? Why throw that information away?
> > Also no Documentation/ABI/ update for your new userspace api that you > > just created? > > > > should that be a comment in the .h?
No, you need a description in Documentation/ABI/ about any sysfs/configfs/securityfs/whatever for new things you are creating.
thanks,
greg k-h
| |