Em Wed, Jul 07, 2021 at 11:16:20AM +0530, kajoljain escreveu:
> On 7/7/21 12:45 AM, Arnaldo Carvalho de Melo wrote:
> > Em Tue, Jul 06, 2021 at 05:26:12PM +0530, kajoljain escreveu:
> >> On 6/29/21 12:39 PM, kajoljain wrote:
> >>> On 6/28/21 8:19 PM, Paul A. Clarke wrote:
> >>>> On Mon, Jun 28, 2021 at 11:53:41AM +0530, Kajol Jain wrote:
> >>>>> @@ -713,7 +711,16 @@ static void set_regs_in_dict(PyObject *dict,
> >>>>> struct evsel *evsel)
> >>>>> {
> >>>>> struct perf_event_attr *attr = &evsel->core.attr;
> >>>>> - char bf[512];
> >>>>> +
> >>>>> + /*
> >>>>> + * Here value 28 is a constant size which can be used to print
> >>>>> + * one register value and its corresponds to:
> >>>>> + * 16 chars is to specify 64 bit register in hexadecimal.
> >>>>> + * 2 chars is for appending "0x" to the hexadecimal value and
> >>>>> + * 10 chars is for register name.
> >>>>> + */
> >>>>> + int size = __sw_hweight64(attr->sample_regs_intr) * 28;
> >>>>> + char bf[size];
> >>>> I propose using a template rather than a magic number here. Something
> >>>> like:
> >>>> const char reg_name_tmpl[] = "10 chars ";
> >>>> const char reg_value_tmpl[] = "0x0123456789abcdef";
> >>>> const int size = __sw_hweight64(attr->sample_regs_intr) +
> >>>> sizeof reg_name_tmpl + sizeof reg_value_tmpl;
> >>> Thanks for reviewing the patch. Yes these are
> >>> some standardization we can do by creating macros for different
> >>> fields.
> >>> The basic idea is, we want to provide significant buffer size
> >>> based on number of registers present in sample_regs_intr to accommodate
> >>> all data.
> >> Is the approach used in this patch looks fine to you?
> > Yeah, and the comment you provide right above it explains it, so I think
> > that is enough, ok?
> Thanks for reviewing it. As you said added comment already explains
> why we are taking size constant as 28, should we skip adding macros part?
> Can you pull this patch.
Sure.
- Arnaldo
