Messages in this thread | | | Subject | Re: [PATCH 10/21] EDAC, ghes: Remove pvt->detail_location string | From | James Morse <> | Date | Wed, 29 May 2019 16:13:20 +0100 |
| |
Hi Robert,
On 29/05/2019 09:44, Robert Richter wrote: > The detail_location[] string in struct ghes_edac_pvt is complete > useless and data is just copied around. Put everything into > e->other_detail from the beginning.
We still print all that complete-useless detail_location stuff... so this commit message had me confused about what you're doing here. I think you meant the space for the string, instead of the value!
| detail_location[] is used to collect two location strings so they can be passed as one | to trace_mc_event(). Instead of having an extra copy step, assemble the location string | in other_detail[] from the beginning.
> diff --git a/drivers/edac/ghes_edac.c b/drivers/edac/ghes_edac.c > index 39702bac5eaf..c18f16bc9e4d 100644 > --- a/drivers/edac/ghes_edac.c > +++ b/drivers/edac/ghes_edac.c > @@ -23,8 +23,7 @@ struct ghes_edac_pvt { > struct mem_ctl_info *mci; > > /* Buffers for the error handling routine */ > - char detail_location[240]; > - char other_detail[160]; > + char other_detail[400]; > char msg[80]; > }; > > @@ -225,13 +224,14 @@ void ghes_edac_report_mem_error(int sev, struct cper_sec_mem_err *mem_err) > memset(e, 0, sizeof (*e)); > e->error_count = 1; > strcpy(e->label, "unknown label"); > - e->msg = pvt->msg; > - e->other_detail = pvt->other_detail; > e->top_layer = -1; > e->mid_layer = -1; > e->low_layer = -1; > - *pvt->other_detail = '\0'; > + e->msg = pvt->msg; > + e->other_detail = pvt->other_detail; > + > *pvt->msg = '\0'; > + *pvt->other_detail = '\0';
... so no change? Could you drop this hunk?
Regardless, Reviewed-by: James Morse <james.morse@arm.com>
Thanks,
James
| |