From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from nx201.node01.secure-mailgate.com (nx201.node01.secure-mailgate.com [89.22.108.201]) by sourceware.org (Postfix) with ESMTPS id 552803858438 for ; Mon, 18 Jul 2022 19:06:10 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 552803858438 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=trande.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=trande.de Received: from host202.checkdomain.de ([185.137.168.148]) by node01.secure-mailgate.com with esmtps (TLSv1.2:AES128-GCM-SHA256:128) (Exim 4.92) (envelope-from ) id 1oDW4C-00322x-1v; Mon, 18 Jul 2022 21:06:07 +0200 X-SecureMailgate-Identity: zied.guermazi@trande.de;host202.checkdomain.de Received: from [192.168.178.42] (dynamic-077-182-009-072.77.182.pool.telefonica.de [77.182.9.72]) (Authenticated sender: zied.guermazi@trande.de) by host202.checkdomain.de (Postfix) with ESMTPSA id 2DAC1281C0A; Mon, 18 Jul 2022 21:06:01 +0200 (CEST) X-SecureMailgate-Identity: zied.guermazi@trande.de;host202.checkdomain.de Message-ID: Date: Mon, 18 Jul 2022 21:06:00 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.1 Subject: Re: [PATCH v6 4/7] start/stop btrace with coresight etm and collect etm buffer on linux os Content-Language: en-US To: "Metzger, Markus T" Cc: "gdb-patches@sourceware.org" References: <20210531213307.275079-1-zied.guermazi@trande.de> <20210531213307.275079-5-zied.guermazi@trande.de> <761d9b7e-09f3-7c00-1f8f-df9af83c5aba@trande.de> From: Zied Guermazi In-Reply-To: X-PPP-Message-ID: <20220718190601.1826518.88481@host202.checkdomain.de> X-PPP-Vhost: trande.de X-Originating-IP: 185.137.168.148 X-SecureMailgate-Domain: host202.checkdomain.de X-SecureMailgate-Username: 185.137.168.148 Authentication-Results: secure-mailgate.com; auth=pass smtp.auth=185.137.168.148@host202.checkdomain.de X-SecureMailgate-Outgoing-Class: ham X-SecureMailgate-Outgoing-Evidence: SB/global_tokens (1.98000994096e-05) X-Recommended-Action: accept X-Filter-ID: Pt3MvcO5N4iKaDQ5O6lkdGlMVN6RH8bjRMzItlySaT+/aYYObRTo4TMJaImi/v0QPUtbdvnXkggZ 3YnVId/Y5jcf0yeVQAvfjHznO7+bT5xepKJyYLZISh0ScmnJxtOQL++T0fzDkcbraJCnPSgcQrqW ghmgrvbuYBMbgPFqhEjKF590ee3aGyyzch48U9RYPumU89c8OaxcWLmp7ynpEnXTvTO01BTkMZHu +B0+f2Rkg5BhoNcHfSWweO3uPweQOspDufM3ay4ktP0suaVr4qkN1tR5WGFFI2Zu1mFmj/jWuw/Y Jn6sireiT/nV8ihOURqb7MAH1NrJot1V9507yhTu9n8Q9r3I67biccgDh5f8Hz+Eg3Qmi4KAfpBm FUZXTPwIrPXd2Nf3MqpCNcEZABqXSHvjdU6ko1j7gyOKOKF7y9KyMhs+dROCnF0qWM2TLX4i2bdm Lrt/fdDS9Ii3kWdcYUysm0ld2GCJTCinOsqiQcaylmhK3EEc7P+KlnTxQJtR4QxWMOifmt9LOcQX 1afTF2zzHOUj1s00F3rRRpqf2Enr8123wW1PIBoO0QaQo4b0r//Ut6pPpczZ8zP+0qdA6ZrmnK8d nrZIOzPOFCMHPCNOxUxXI6XX1/yXK/O3SA6y1K1EOm1MYByprvQ6e6ADS3r2wjjEGtaal+lgW0dK YRw2uzZSa4zLNGy8qUnY7v1vYyCRi03s0e/l06zibHcSpbwKA/DsSgtEvE/FmwCAUuNAukfKnqWf +PWMOlMhSYJHRHso2yGjpoJSangIKJxRcEhdPyRFVzp2ngApVWxOJ0wcDGrO2xhBTAhrqRk9Kxv/ HqW5k8JD4GMM1a+xH+p/fYdEAhcvFLGNAi49Bl4QCSPcD4DJ/3SVcATUovrLfIqu8ivwIyvWNWKr Cx6+8uEPs/B5FR2DxQ26rHITgGPNMQqF4dRRuuH+kKQ8806Pa8l84j4yYnhyduAtV+uhJ/DV9vjD SqX3icbH6/Gp21JrsAi7BjuxQV/ck9inbb5tHHASJNUmoOHSoqgqxfHmWQOr3l2HwNBzdZAEeW6A 0mBj6kqhGciiFNfzGVxCyoOoXofymrrMnZyIPBD8J+xJbitKkPOMS2N9fz44eXhzJTr44Hm1dtH7 fJI8+wFmHvZn0mGxpMnBWCb54Omnhy/euE49L580IwHza2T+59S3hng8SulA1b1ymd5KQt+40IiQ Mxf9sFUNghHbOjYSFtxREjppmVhT/W3/qRy6LktaKsD2OKHH5lr9xXvSM4nM3avg X-Report-Abuse-To: spam@node04.secure-mailgate.com X-Spam-Status: No, score=2.0 required=5.0 tests=BAYES_00, BODY_8BITS, FOREIGN_BODY1, HTML_MESSAGE, JMQ_SPF_NEUTRAL, KAM_DMARC_STATUS, NICE_REPLY_A, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=no autolearn_force=no version=3.4.6 X-Spam-Level: * X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Content-Filtered-By: Mailman/MimeDel 2.1.29 X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 18 Jul 2022 19:06:16 -0000 hello Markus, the proposal solves the issue for scoped_fd, we can use it and create a second scoped_fd using the returned file descriptor since scoped_fd has this constructor explicit scoped_fd (int fd = -1) noexcept : m_fd (fd) {}. for scoped_mmap we can not do the same since the class is missing a constructor taking void *mem and size_t length as arguments. will it be fine to add such a constructor to the class? will it be enough to assign the memory pointer and the length to the class members? Kind Regards Zied Guermazi On 20.06.22 14:52, Metzger, Markus T wrote: > > Hello Zied, > > /* open the scoped fd. */ > > static scoped_fd * > perf_event_open ( const struct perf_event_attr *event_attributes, > const int pid) > > The purpose of those scoped_ classes is to allocated objects on the > stack to have them destroy their content in case of exceptions. > > In the above example, perf_event_open() may use a scoped_fd internally > but would return the actual file descriptor after releasing it.  The > caller may put the returned file descriptor into its own scoped_fd > object on its stack. > > regards, > > markus. > > /* open the scoped fd. */ > > static scoped_fd * > perf_event_open ( const struct perf_event_attr *event_attributes, > const int pid) > > /*Open the scoped mmap data */ > > static scoped_mmap* > perf_event_mmap_data (const scoped_fd* fd, size_t* size, size_t page_size) > > and > > /*Open the scoped mmap aux */ > > static scoped_mmap* > perf_event_mmap_aux (const scoped_fd* fd, struct perf_event_mmap_page > *header, size_t* size, size_t page_size) > > and call them within linux_enable_bts, linux_enable_pt and > linux_enable_etm > > I tried two alternatives and both of them failed: > > - Alternative 1: instantiate the returned scoped_mmap on the stack of > the perf_event_mmap_data and perf_event_mmap_aux: the compiler refused > to assign the scoped_mmap variables and they will not be valid anymore > in the linux_enable_bts, linux_enable_pt and linux_enable_etm > > - Alternative 2: instantiate the returned scoped_mmap on the heap by > allocating them. the compiler compiles but, once we stop the tracing > we ca can not start it again cause the resources are busy. > > for the time being I will put this refactoring action on hold. > > Any idea or support to progress further are welcome > > Kind Regards > > Zied Guermazi > > On 13.05.22 07:31, Metzger, Markus T wrote: > > Hello Zied, > > +cs_etm_get_register (int cpu, const char *path) > > +{ > > [...] > > + > > +  uint32_t val = 0; > > + > > +  int  found = fscanf (file.get (), "0x%x", &val); > > +  if (found != 1) > > +    error (_("Failed to read coresight register from %s."), filename); > > +  return val; > > > > [Zied] existing code in this file has an empty line before the > last return everywhere. shall I stick to this convention? > eliminating the empty lines before returns will bring > inconsistency in the file. which coding convention do we have to > apply here? > > > Yes, let’s stick to that and add the empty line. > > so both perf_event_mmap_data perf_event_mmap_aux can be in fact > reduced to one function if we give size_t size, size_t page_size, > int offset as parameters. > > BTS allocates the data buffer including the header, whereas PT > allocates the aux buffer plus the header.  We can still make it > one function, of course, but I don’t think that it will be any > easier to read that way. > > 2- how to bring this change to the mainstream. here we have two > alternatives : either I do this restructuring for etm only and > then you take care of using the helper functions for bts and pt or > I commit it as it is and then I issue a patch for this point only > for bts PT and ETM. What do you prefer? > > Ideally, we’d first restructure the existing code, then add the > new use.  Are you able to test BTS and PT? > > Regards, > > Markus. > > *From:* Zied Guermazi > > *Sent:* Friday, May 13, 2022 12:52 AM > *To:* Metzger, Markus T > ; gdb-patches@sourceware.org > *Subject:* Re: [PATCH v6 4/7] start/stop btrace with coresight etm > and collect etm buffer on linux os > > Hello Markus, > > thanks for your feedback, below are the reworking comments. > > /Zied > > On 23.06.21 10:00, Metzger, Markus T wrote: > > Hello Zied, > > > > This patch implement the lower layer for starting ad stopping > > ARM CoreSight tracing on linux targets for arm and aarch64 > > > > The patch looks good overall.  There are a few style nits and I'd ask you to > > split the PAGE_SIZE changes into a separate patch as they are unrelated. > > > > Then, there's the discussion about sharing perf_event buffer mapping. > > I pointed out which parts I believe can be shared. > > > > > > +/* Teardown branch tracing.  */ > > + > > +void > > +arm_linux_nat_target::teardown_btrace (struct btrace_target_info *tinfo) > > +{ > > +  /* Ignore errors.  */ > > +  linux_disable_btrace (tinfo); > > +} > > + > > +enum btrace_error > > +arm_linux_nat_target::read_btrace (struct btrace_data *data, > > +                           struct btrace_target_info *btinfo, > > +                           enum btrace_read_type type) > > +{ > > +  return linux_read_btrace (data, btinfo, type); > > +} > > + > > +/* See to_btrace_conf in target.h.  */ > > + > > +const struct btrace_config * > > +arm_linux_nat_target::btrace_conf (const struct btrace_target_info *btinfo) > > +{ > > +  return linux_btrace_conf (btinfo); > > +} > > > > There's some inconsistency in comments on functions ranging from no comment > > over referring to the original target struct, to an own comment. > > [Zied] I will align the comments. please notice that the same > applies to x86-linux-nat.c (it was a copy-paste from it) > > > > > > > > > @@ -483,10 +487,11 @@ linux_enable_bts (ptid_t ptid, const struct > > btrace_config_bts *conf) > >   scoped_fd fd (syscall (SYS_perf_event_open, &bts->attr, pid, -1, -1, 0)); > >   if (fd.get () < 0) > >     diagnose_perf_event_open_fail (); > > +  long page_size = sysconf (_SC_PAGESIZE); > > > > Please split those PAGE_SIZE changes into a separate patch.  This is unrelated > > to what this patch is doing. > > > > Note that PAGE_SIZE was unsigned whereas sysconf () returns a signed integer. > > I'd expect compilers to require proper casting. > > [Zied] done > > > > > > > > > +/* Enable ARM CoreSight ETM tracing.  */ > > + > > +static struct btrace_target_info * > > +linux_enable_etm (ptid_t ptid, const struct btrace_config_etm *conf) > > +{ > > [...] > > +  etm->attr.sample_type = PERF_SAMPLE_CPU; > > +  etm->attr.read_format = PERF_FORMAT_ID; > > +  etm->attr.sample_id_all = 1; > > > > You enable sampling.  Wouldn't you need to mmap the data buffer, as well? > > [Zied] it is not needed for current implementation. removed. > > > > > > > > > This ... > > > > +  errno = 0; > > +  scoped_fd fd (syscall (SYS_perf_event_open, &etm->attr, pid, -1, -1, 0)); > > +  if (fd.get () < 0) > > +    diagnose_perf_event_open_fail (); > > + > > +  /* Allocate the configuration page.  */ > > +  long page_size = sysconf (_SC_PAGESIZE); > > +  scoped_mmap data (nullptr, page_size, PROT_READ | PROT_WRITE, > > MAP_SHARED, > > +             fd.get (), 0); > > +  if (data.get () == MAP_FAILED) > > +    error (_("Failed to map trace user page: %s."), safe_strerror (errno)); > > + > > +  struct perf_event_mmap_page *header = (struct perf_event_mmap_page *) > > +    data.get (); > > + > > +  header->aux_offset = header->data_offset + header->data_size; > > +  /* Convert the requested size in bytes to pages (rounding up).  */ > > +  pages = ((size_t) conf->size / page_size > > +     + ((conf->size % page_size) == 0 ? 0 : 1)); > > +  /* We need at least one page.  */ > > +  if (pages == 0) > > +    pages = 1; > > + > > +  /* The buffer size can be requested in powers of two pages.  Adjust PAGES > > +     to the next power of two.  */ > > +  for (pg = 0; pages != ((size_t) 1 << pg); ++pg) > > +    if ((pages & ((size_t) 1 << pg)) != 0) > > +      pages += ((size_t) 1 << pg); > > + > > +  /* We try to allocate the requested size. > > +     If that fails, try to get as much as we can.  */ > > +  scoped_mmap aux; > > +  for (; pages > 0; pages >>= 1) > > +    { > > +      size_t length; > > +      __u64 data_size; > > +      data_size = (__u64) pages * page_size; > > + > > +      /* Don't ask for more than we can represent in the configuration.  */ > > +      if ((__u64) UINT_MAX < data_size) > > +  continue; > > + > > +      length = (size_t) data_size; > > + > > +      /* Check for overflows.  */ > > +      if ((__u64) length != data_size) > > +  continue; > > + > > +      header->aux_size = data_size; > > + > > +      errno = 0; > > +      aux.reset (nullptr, length, PROT_READ, MAP_SHARED, fd.get (), > > +          header->aux_offset); > > +      if (aux.get () != MAP_FAILED) > > +  break; > > +    } > > +  if (pages == 0) > > +    error (_("Failed to map trace buffer: %s."), safe_strerror (errno)); > > + > > +  etm->etm.size = aux.size (); > > +  etm->etm.mem = (const uint8_t *) aux.release (); > > +  etm->etm.data_head = &header->aux_head; > > +  etm->etm.last_head = header->aux_tail; > > +  etm->header = (struct perf_event_mmap_page *) data.release (); > > +  gdb_assert (etm->header == header); > > > > ... can be shared with btrace_enable_pt () by introducing some > > > > perf_event_open_aux (struct perf_event_buffer *, const struct perf_event_attr *) > > > > helper. > > > > And if you indeed need to mmap the data buffer, as well, we can share that with > > btrace_enable_bts (), although we'd need some more restructuring to leave > > perf_event_open to the caller and just allocate the data and aux buffers using > > two helpers - they would again look very similar but need to touch a different > > set of fields in the header, so I'd keep those separate. > > > > btrace_enable_foo () would then become > > { > >   perf_event_open () > >   perf_event_mmap_data () > >   perf_event_mmap_aux ()  /* not for bts */ > > } > > > > [Zied] I like the idea, there are two aspects that we need to > consider to bring it to the mainstream code: > > - 1: interface and scope definition > > static scoped_fd perf_event_open ( const struct perf_event_attr > *event_attributes,  const int pid ) > > { > >   errno = 0; >   scoped_fd fd (syscall (SYS_perf_event_open, event_attributes, > pid, -1, -1, 0)); >   if (fd.get () < 0) >     diagnose_perf_event_open_fail (); > >   return fd; > > } > > this function will only open the file descriptor and return it > > static scoped_mmap data perf_event_mmap_data (const scoped_fd fd, > size_t size, size_t page_size, int offset) > > { > >  //alternative 1 just create it and return it > >   /* Allocate the configuration page. */ >   scoped_mmap data (nullptr, page_size, PROT_READ | PROT_WRITE, > MAP_SHARED, >             fd.get (), 0); >   if (data.get () == MAP_FAILED) >     error (_("Failed to map trace user page: %s."), safe_strerror > (errno)); > >   return data; > >  //alternative 2, create it it and make sure that we resize it to > the highest possible power of 2 supported by the system > > /* Convert the requested size in bytes to pages (rounding up).  */ >   pages = size / page_size >        + (size % page_size) == 0 ? 0 : 1)); >   /* We need at least one page.  */ >   if (pages == 0) >     pages = 1; > >   /* The buffer size can be requested in powers of two pages.  > Adjust PAGES >      to the next power of two.  */ >   for (pg = 0; pages != ((size_t) 1 << pg); ++pg) >     if ((pages & ((size_t) 1 << pg)) != 0) >       pages += ((size_t) 1 << pg); > >   /* We try to allocate the requested size. >      If that fails, try to get as much as we can.  */ >   scoped_mmap data; >   for (; pages > 0; pages >>= 1) >     { >       size_t length; >       __u64 data_size; > >       data_size = (__u64) pages * page_size; > >       /* Don't ask for more than we can represent in the > configuration.  */ >       if ((__u64) UINT_MAX < data_size) >     continue; > >       size = (size_t) data_size; >       length = size + page_size; > >       /* Check for overflows.  */ >       if ((__u64) length != data_size + page_size) >     continue; > >       errno = 0; >       /* The number of pages we request needs to be a power of > two.  */ >       data.reset (nullptr, length, PROT_READ, MAP_SHARED, fd.get > (), offset); >       if (data.get () != MAP_FAILED) >     break; >     } > >   if (pages == 0) >     error (_("Failed to map trace buffer: %s."), safe_strerror > (errno)); > > } > > static scoped_mmap perf_event_mmap_aux (const scoped_fd fd, size_t > size, size_t page_size, int offset) > > { > > //idem, the function is similar to previous one it is only the > offset in data.reset call that changes > > } > > so both perf_event_mmap_data perf_event_mmap_aux can be in fact > reduced to one function if we give size_t size, size_t page_size, > int offset as parameters. > > 2- how to bring this change to the mainstream. here we have two > alternatives : either I do this restructuring for etm only and > then you take care of using the helper functions for bts and pt or > I commit it as it is and then I issue a patch for this point only > for bts PT and ETM. What do you prefer? > > > > +  length = fread (buffer, 1, length, file.get ()); > > +  buffer[length]='\0'; > > > > Spaces around =. > > [Zied] done. > > > > > > > +  while ((--length) != 0) > > +    { > > +      if ((buffer[length] == ',') || (buffer[length] == '-')) > > +  { > > +    length++; > > +    break; > > +  } > > +    } > > + > > +  int cpu_count; > > +  int found = sscanf (&buffer[length], "%d", &cpu_count); > > +  if (found < 1) > > +    error (_("Failed to get cpu count in %s: %s."), > > +       buffer, safe_strerror (errno)); > > + > > +  cpu_count ++; > > +  return (cpu_count); > > > > No need for (). > > [Zied] done. > > > > > > > > > +  char filename[PATH_MAX]; > > +  snprintf (filename, PATH_MAX, > > > > sizeof (filename) > > [Zied] done. > > > > > > > +  char filename[PATH_MAX]; > > + > > +  /* Get coresight register from sysfs.  */ > > +  snprintf (filename, PATH_MAX, > > > > sizeof (filename) > > [Zied] done > > > > > > > +      "/sys/bus/event_source/devices/cs_etm/cpu%d/%s", cpu, path); > > +  errno = 0; > > +  gdb_file_up file = gdb_fopen_cloexec (filename, "r"); > > +  if (file.get () == nullptr) > > +    error (_("Failed to open %s: %s."), filename, safe_strerror (errno)); > > + > > +  uint32_t val = 0; > > + > > +  int  found = fscanf (file.get (), "0x%x", &val); > > +  if (found != 1) > > +    error (_("Failed to read coresight register from %s."), filename); > > +  return val; > > +} > > > > Empty line before return?  I'd also remove the empty line between the > > declaration of val and the call to fscanf (). > > > > There are several very similar functions in this patch and each is structured > > differently: > > > > +perf_event_etm_event_type () > > +{ > > [...] > > + > > +  int type, found = fscanf (file.get (), "%d", &type); > > +  if (found != 1) > > +    error (_("Failed to read the ETM event type from %s."), filename); > > + > > +  return type; > > > > +get_cpu_count (void) > > +{ > > [...] > > + > > +  int cpu_count; > > +  int found = sscanf (&buffer[length], "%d", &cpu_count); > > +  if (found < 1) > > +    error (_("Failed to get cpu count in %s: %s."), > > +             buffer, safe_strerror (errno)); > > + > > +  cpu_count ++; > > +  return (cpu_count); > > > > +perf_event_etm_event_sink (const struct btrace_config_etm *conf) > > +{ > > [...] > > + > > +  unsigned int sink; > > +  int  found = fscanf (file.get (), "0x%x", &sink); > > +  if (found != 1) > > +    error (_("Failed to read the ETM sink from %s."), filename); > > + > > +  return sink; > > > > +cs_etm_get_register (int cpu, const char *path) > > +{ > > [...] > > + > > +  uint32_t val = 0; > > + > > +  int  found = fscanf (file.get (), "0x%x", &val); > > +  if (found != 1) > > +    error (_("Failed to read coresight register from %s."), filename); > > +  return val; > > > > [Zied] existing code in this file has an empty line before the > last return everywhere. shall I stick to this convention? > eliminating the empty lines before returns will bring > inconsistency in the file. which coding convention do we have to > apply here? > > > > > + > > +#define CORESIGHT_ETM_PMU_SEED  0x10 > > + > > +/* Calculate trace_id for this cpu > > +   to be kept aligned with coresight-pmu.h.  */ > > + > > +static inline int > > +coresight_get_trace_id (int cpu) > > +{ > > +  return (CORESIGHT_ETM_PMU_SEED + (cpu * 2)); > > > > In patch 3, you wrote > > > > +  /* Trace id for this thread. > > +     On a linux system, trace_id is assigned per cpu. The kernel copies > > +     the traces of each thread in a dedicated ring buffer. By this, > > +     traces belonging to different threads are de-multiplexed. > > +     On an RTOS system, especially when routing the traces outside of the SoC, > > +     the OS has no other mean for de-multiplexing the traces than > > +     the trace_id. The hardware (ETM IP) reserves 7 bits for the trace_id. > > +     On linux system trace id is not needed, set it to 0xFF to ignore it > > +     during parsing.  */ > > +  uint8_t trace_id; > > > > Should this function return uint8_t and check that the ID is 7 bit max? > > [Zied] done, reserved and not allowed values are also generating a > warning now. > > > > > > > > > +static void > > +fill_etm_trace_params (struct cs_etm_trace_params *etm_trace_params, int > > cpu) > > +{ > > +  if (cs_etm_is_etmv4 (cpu) == true) > > > > No need for explicit checks on bool. > > [Zied] done. > > > > > > > > > +static void > > +linux_fill_btrace_etm_config (struct btrace_target_info *tinfo, > > +                        struct btrace_data_etm_config *conf) > > +{ > > + > > +  cs_etm_trace_params etm_trace_params; > > > > Please declare at initialization time. > > [Zied] pushed forwards before the for loop > > > > > > > +  conf->cpu_count = get_cpu_count (); > > +  conf->etm_trace_params = new std::vector; > > +  for (int i = 0; i < conf->cpu_count; i++) > > +    { > > +      fill_etm_trace_params (&etm_trace_params,i); > > +      conf->etm_trace_params->push_back (etm_trace_params); > > +    } > > > > We need to avoid leaking the vector when fill_etm_trace_params () throws. > > > > > > > > +static enum btrace_error > > +linux_read_etm (struct btrace_data_etm *btrace, > > +         struct btrace_target_info *tinfo, > > +         enum btrace_read_type type) > > +{ > > +  struct perf_event_buffer *etm; > > +  etm = &tinfo->variant.etm.etm; > > > > Please combine.  No forward declarations anymore.  The old code was written > > when GDB was still C. > > [Zied] done. > > > > > > > > > regards, > > markus. > > Intel Deutschland GmbH > > Registered Address: Am Campeon 10, 85579 Neubiberg, Germany > > Tel: +49 89 99 8853-0,www.intel.de > > Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva > > Chairperson of the Supervisory Board: Nicole Lau > > Registered Office: Munich > > Commercial Register: Amtsgericht Muenchen HRB 186928 > > > > -- > > > *Zied Guermazi* > founder > > Trande GmbH > Leuschnerstraße 2 > 69469 Weinheim/Germany > > Mobile: +491722645127 > mailto:zied.guermazi@trande.de > > *Trande GmbH* > Leuschnerstraße 2, D-69469 Weinheim; Telefon: +491722645127 > Sitz der Gesellschaft: Weinheim- Registergericht: AG Mannheim HRB > 736209 - Geschäftsführung: Zied Guermazi > > *Confidentiality Note* > This message is intended only for the use of the named > recipient(s) and may contain confidential and/or privileged > information. If you are not the intended recipient, please contact > the sender and delete the message. Any unauthorized use of the > information contained in this message is prohibited. > > Intel Deutschland GmbH > Registered Address: Am Campeon 10, 85579 Neubiberg, Germany > Tel: +49 89 99 8853-0, www.intel.de > Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany > Doon Silva > Chairperson of the Supervisory Board: Nicole Lau > Registered Office: Munich > Commercial Register: Amtsgericht Muenchen HRB 186928 > > Intel Deutschland GmbH > Registered Address: Am Campeon 10, 85579 Neubiberg, Germany > Tel: +49 89 99 8853-0, www.intel.de > Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva > Chairperson of the Supervisory Board: Nicole Lau > Registered Office: Munich > Commercial Register: Amtsgericht Muenchen HRB 186928 > -- *Zied Guermazi* founder Trande GmbH Leuschnerstraße 2 69469 Weinheim/Germany Mobile: +491722645127 mailto:zied.guermazi@trande.de *Trande GmbH* Leuschnerstraße 2, D-69469 Weinheim; Telefon: +491722645127 Sitz der Gesellschaft: Weinheim- Registergericht: AG Mannheim HRB 736209 - Geschäftsführung: Zied Guermazi *Confidentiality Note* This message is intended only for the use of the named recipient(s) and may contain confidential and/or privileged information. If you are not the intended recipient, please contact the sender and delete the message. Any unauthorized use of the information contained in this message is prohibited.