On Fri, Oct 26, 2018 at 08:32:16PM +0100, Kees Cook wrote:
> On Fri, Oct 26, 2018 at 7:00 PM, Joel Fernandes (Google)
> <j...@joelfernandes.org> wrote:
> > Both the id and type fields of a pstore_record are set by
> > ramoops_get_next_prz. So we can just pass a pointer to the pstore_record
> > instead of passing individual elements. This results in cleaner more
> > readable code and fewer lines.
> >
> > Signed-off-by: Joel Fernandes (Google) <j...@joelfernandes.org>
> > ---
> > fs/pstore/ram.c | 18 ++++++++----------
> > 1 file changed, 8 insertions(+), 10 deletions(-)
> >
> > diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
> > index 3055e05acab1..710c3d30bac0 100644
> > --- a/fs/pstore/ram.c
> > +++ b/fs/pstore/ram.c
> > @@ -125,7 +125,7 @@ static int ramoops_pstore_open(struct pstore_info *psi)
> >
> > static struct persistent_ram_zone *
> > ramoops_get_next_prz(struct persistent_ram_zone *przs[], uint *c,
> > - u64 *id, enum pstore_type_id *typep, bool update)
> > + struct pstore_record *record, bool update)
> > {
> > struct persistent_ram_zone *prz;
> > int i = (*c)++;
> > @@ -145,8 +145,8 @@ ramoops_get_next_prz(struct persistent_ram_zone
> > *przs[], uint *c,
> > if (!persistent_ram_old_size(prz))
> > return NULL;
> >
> > - *typep = prz->type;
> > - *id = i;
> > + record->type = prz->type;
> > + record->id = i;
>
> Yes yes. I've been meaning to get all this cleaned up after I
> refactored everything to actually HAVE record at all. :P
>
> >
> > return prz;
> > }
> > @@ -254,7 +254,7 @@ static ssize_t ramoops_pstore_read(struct pstore_record
> > *record)
> > /* Find the next valid persistent_ram_zone for DMESG */
> > while (cxt->dump_read_cnt < cxt->max_dump_cnt && !prz) {
> > prz = ramoops_get_next_prz(cxt->dprzs, &cxt->dump_read_cnt,
> > - &record->id, &record->type, 1);
> > + record, 1);
>
> In another patch, I think you could drop the "update" field too, and
> use the record->type instead to determine if update is needed. Like:
>
> static struct persistent_ram_zone *
> ramoops_get_next_prz(struct persistent_ram_zone *przs[], uint c,
> struct pstore_record *record)
> {
> bool update = (record->type == PSTORE_TYPE_DMESG);
> ...
Yes, I agree, I'll do that :)
thanks!
- Joel
