[1/9] Adapt `info probes' to support printing probes of different types.

  A "probe type" (backend for the probe abstraction implemented in probe.[ch])
can extend the information printed by `info probes' by defining additional
columns.  This means that when `info probes' is used to print all the probes
regardless of their types, some of the columns will be "not applicable" to
some of the probes (like, say, the Semaphore column only makes sense for
SystemTap probes).  This patch makes `info probes' fill these slots with "n/a"
marks (currently it breaks the table) and not include headers for which no
actual probe has been found in the list of defined probes.

gdb:

2014-09-25  Jose E. Marchesi  <jose.marchesi@oracle.com>

	* probe.c (print_ui_out_not_applicables): New function.
        (get_num_probes_with_pops): Likewise.
        (info_probes_for_ops): Do not include column headers for probe
        types for which no probe has been actually found on any object.
        Also invoke `print_ui_out_not_applicables' in order to match the
        column rows with the header when probes of several types are
        listed.
---
 gdb/ChangeLog |   10 +++++++++
 gdb/probe.c   |   63 +++++++++++++++++++++++++++++++++++++++++++++++++++------
 2 files changed, 67 insertions(+), 6 deletions(-)
  

On Friday, September 26 2014, Jose E. Marchesi wrote:

> A "probe type" (backend for the probe abstraction implemented in probe.[ch])
> can extend the information printed by `info probes' by defining additional
> columns.  This means that when `info probes' is used to print all the probes
> regardless of their types, some of the columns will be "not applicable" to
> some of the probes (like, say, the Semaphore column only makes sense for
> SystemTap probes).  This patch makes `info probes' fill these slots with "n/a"
> marks (currently it breaks the table) and not include headers for which no
> actual probe has been found in the list of defined probes.

Thanks for the patch, Jose.  Comments below.

> gdb:
>
> 2014-09-25  Jose E. Marchesi  <jose.marchesi@oracle.com>
>
> 	* probe.c (print_ui_out_not_applicables): New function.
>         (get_num_probes_with_pops): Likewise.
>         (info_probes_for_ops): Do not include column headers for probe
>         types for which no probe has been actually found on any object.
>         Also invoke `print_ui_out_not_applicables' in order to match the
>         column rows with the header when probes of several types are
>         listed.
> ---
>  gdb/ChangeLog |   10 +++++++++
>  gdb/probe.c   |   63 +++++++++++++++++++++++++++++++++++++++++++++++++++------
>  2 files changed, 67 insertions(+), 6 deletions(-)
>
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index dbd222d..7cc4f00 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,13 @@
> +2014-09-25  Jose E. Marchesi  <jose.marchesi@oracle.com>
> +
> +	* probe.c (print_ui_out_not_applicables): New function.
> +	(get_num_probes_with_pops): Likewise.
> +	(info_probes_for_ops): Do not include column headers for probe
> +	types for which no probe has been actually found on any object.
> +	Also invoke `print_ui_out_not_applicables' in order to match the
> +	column rows with the header when probes of several types are
> +	listed.
> +

BTW, we usually don't include ChangeLog diff's in the message, because
they make it hard to apply the patch after some time (even hours!).  I
am mentioning this because I had several conflicts when I applied yours
here ;-).  (Of course, one can always delete the ChangeLog diff's from
the patches before applying them, but it is also annoying).

> diff --git a/gdb/probe.c b/gdb/probe.c
> index 859e6e7..5458372 100644
> --- a/gdb/probe.c
> +++ b/gdb/probe.c
> @@ -411,6 +411,33 @@ gen_ui_out_table_header_info (VEC (bound_probe_s) *probes,
>    do_cleanups (c);
>  }
>  
> +/* Helper function to print not-applicable strings for all the extra
> +   columns defined in a probe_ops.  */
> +
> +static void
> +print_ui_out_not_applicables (const struct probe_ops *pops)
> +{
> +  struct cleanup *c;
> +  VEC (info_probe_column_s) *headings = NULL;
> +  info_probe_column_s *column;
> +  int ix;
> +  
> +  if (pops->gen_info_probes_table_header == NULL)
> +    return;
> +
> +  c = make_cleanup (VEC_cleanup (info_probe_column_s), &headings);
> +  pops->gen_info_probes_table_header (&headings);
> +  
> +  for (ix = 0;
> +       VEC_iterate (info_probe_column_s, headings, ix, column);
> +       ++ix)
> +    {
> +      ui_out_field_string (current_uiout, column->field_name, _("n/a"));
> +    }

You can remove the brackets, since it's a one-statement for.

> +  
> +  do_cleanups (c);
> +}
> +
>  /* Helper function to print extra information about a probe and an objfile
>     represented by PROBE.  */
>  
> @@ -483,6 +510,23 @@ get_number_extra_fields (const struct probe_ops *pops)
>    return n;
>  }
>  
> +/* Helper function that returns the number of probes in PROBES having
> +   the given POPS.  */
> +
> +static int
> +get_num_probes_with_pops (VEC (bound_probe_s) *probes,
> +			  const struct probe_ops *pops)
> +{
> +  int res = 0;
> +  struct bound_probe *probe;
> +  int ix;
> +
> +  for (ix = 0; VEC_iterate (bound_probe_s, probes, ix, probe); ++ix)
> +    res += (probe->probe->pops == pops);

Please, remove the parens, we don't use them in this case.

> +
> +  return res;
> +}

Also, this function could be simplified.  You are not using the count
anywhere, so why not just look for the first match of
"probe->probe->pops == pops", and return 1 if it is found (0 otherwise)?
The function name should be adjusted in this case.

> +
>  /* See comment in probe.h.  */
>  
>  void
> @@ -518,6 +562,8 @@ info_probes_for_ops (const char *arg, int from_tty,
>  	}
>      }
>  
> +  probes = collect_probes (objname, provider, probe_name, pops);
> +  
>    if (pops == NULL)
>      {
>        const struct probe_ops *po;
> @@ -530,15 +576,16 @@ info_probes_for_ops (const char *arg, int from_tty,
>  
>  	 To do that, we iterate over all probe_ops, querying each one about
>  	 its extra fields, and incrementing `ui_out_extra_fields' to reflect
> -	 that number.  */
> +	 that number.  But note that we ignore the probe_ops for which no probes
> +         are defined with the given search criteria.  */
>  
>        for (ix = 0; VEC_iterate (probe_ops_cp, all_probe_ops, ix, po); ++ix)
> -	ui_out_extra_fields += get_number_extra_fields (po);
> +	if (get_num_probes_with_pops (probes, po))
> +	  ui_out_extra_fields += get_number_extra_fields (po);

Here is the only place you are doing an implicit comparison.  I was
going to tell you to compare explicitly, i.e., "get_num_probes_with_pops
(probes, po) > 0", but when you implement the simplification I suggested
above, there is no need anymore.

>      }
>    else
>      ui_out_extra_fields = get_number_extra_fields (pops);
>  
> -  probes = collect_probes (objname, provider, probe_name, pops);
>    make_cleanup (VEC_cleanup (probe_p), &probes);

This call to "make_cleanup" should be moved up, too.

>    make_cleanup_ui_out_table_begin_end (current_uiout,
>  				       4 + ui_out_extra_fields,
> @@ -572,10 +619,12 @@ info_probes_for_ops (const char *arg, int from_tty,
>        const struct probe_ops *po;
>        int ix;
>  
> -      /* We have to generate the table header for each new probe type that we
> -	 will print.  */
> +      /* We have to generate the table header for each new probe type
> +	 that we will print.  Note that this excludes probe types not
> +	 having any defined probe with the search criteria.  */
>        for (ix = 0; VEC_iterate (probe_ops_cp, all_probe_ops, ix, po); ++ix)
> -	gen_ui_out_table_header_info (probes, po);
> +	if (get_num_probes_with_pops (probes, po) > 0)
> +	  gen_ui_out_table_header_info (probes, po);
>      }
>    else
>      gen_ui_out_table_header_info (probes, pops);
> @@ -605,6 +654,8 @@ info_probes_for_ops (const char *arg, int from_tty,
>  	       ++ix)
>  	    if (probe->probe->pops == po)
>  	      print_ui_out_info (probe->probe);
> +	    else if (get_num_probes_with_pops (probes, po) > 0)
> +	      print_ui_out_not_applicables (po);

Hm, this loop is kind of confusing.  I have a few ideas to make it
clearer, but that's for another patch :-).

>  	}

Both places will need to be adjusted when you simplify the
get_num_probes_with_pops function.
  

Hi Sergio.
    
    > A "probe type" (backend for the probe abstraction implemented in probe.[ch])
    > can extend the information printed by `info probes' by defining additional
    > columns.  This means that when `info probes' is used to print all the probes
    > regardless of their types, some of the columns will be "not applicable" to
    > some of the probes (like, say, the Semaphore column only makes sense for
    > SystemTap probes).  This patch makes `info probes' fill these slots with "n/a"
    > marks (currently it breaks the table) and not include headers for which no
    > actual probe has been found in the list of defined probes.
    
    Thanks for the patch, Jose.  Comments below.

I agree/comply with all these comments :)