Messages in this thread |  | | | From | Arnd Bergmann <> | | Subject | Re: [PATCH 1/2] ramoops: use pstore interface | | Date | Thu, 17 Nov 2011 15:07:00 +0000 |
| |
On Wednesday 16 November 2011, Kees Cook wrote: > Instead of using /dev/mem directly, use the common pstore infrastructure > to handle Oops gathering and extraction. > > Signed-off-by: Kees Cook <keescook@chromium.org>
Sounds like a very good plan to me. It probably makes sense to move the entire driver into fs/pstore after this. Otherwise, I have only trivial style comments:
> +static int ramoops_pstore_open(struct pstore_info *psi); > +static int ramoops_pstore_close(struct pstore_info *psi); > +static ssize_t ramoops_pstore_read(u64 *id, enum pstore_type_id *type, > + struct timespec *time, > + char **buf, > + struct pstore_info *psi); > +static int ramoops_pstore_write(enum pstore_type_id type, > + enum kmsg_dump_reason reason, u64 *id, > + unsigned int part, > + size_t size, struct pstore_info *psi); > +static int ramoops_pstore_erase(enum pstore_type_id type, u64 id, > + struct pstore_info *psi);
Can you do it without forward declarations? Many people find code more readable if it is structure in the natural order that avoids these.
> +static int ramoops_pstore_close(struct pstore_info *psi) > +{ > + return 0; > +}
Do you actually have to provide this if it's empty?
If yes, it might make sense to change the pstore code so that it works without a close function.
Arnd
|  |