Em Wed, Jul 07, 2021 at 11:16:20AM +0530, kajoljain escreveu:

On 7/7/21 12:45 AM, Arnaldo Carvalho de Melo wrote:

quoted

Em Tue, Jul 06, 2021 at 05:26:12PM +0530, kajoljain escreveu:

quoted

On 6/29/21 12:39 PM, kajoljain wrote:

quoted

On 6/28/21 8:19 PM, Paul A. Clarke wrote:

quoted

On Mon, Jun 28, 2021 at 11:53:41AM +0530, Kajol Jain wrote:

quoted

@@ -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];

quoted

quoted

quoted

quoted

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;

quoted

quoted

quoted

   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.

quoted

quoted

   Is the approach used in this patch looks fine to you?

quoted

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