Re: [PATCH 21/38] Add some comments

Kris Van Hees
2024-07-19T20:43:15Z

On Thu, Jun 27, 2024 at 01:38:47AM -0400, eugene.loh@oracle.com wrote:
> From: Eugene Loh <eugene.loh@oracle.com>
>
> Signed-off-by: Eugene Loh <eugene.loh@oracle.com>

Reviewed-by: Kris Van Hees <kris.van.hees@oracle.com>

... with tiny suggestion below

> ---
> libdtrace/dt_prov_uprobe.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/libdtrace/dt_prov_uprobe.c b/libdtrace/dt_prov_uprobe.c
> index 5dbd75e3..5f0c56db 100644
> --- a/libdtrace/dt_prov_uprobe.c
> +++ b/libdtrace/dt_prov_uprobe.c
> @@ -295,6 +295,7 @@ static int provide_probe(dtrace_hdl_t *dtp, const pid_probespec_t *psp,
> /* Mark the provider as a PID-based provider. */
> pvp->pv_flags |= DT_PROVIDER_PID;
>
> + /* Look up or create the underlying probe. */

This seems to contradict the function name. Perhaps rephrase this to say:
"Create and/or lookup the underlying probe."

> uprp = create_underlying(dtp, psp);
> if (uprp == NULL)
> return -1;
> @@ -302,6 +303,7 @@ static int provide_probe(dtrace_hdl_t *dtp, const pid_probespec_t *psp,
> upp = uprp->prv_data;
> upp->flags |= flags;
>
> + /* Look up the overlying probe. */
> prp = dt_probe_lookup(dtp, &pd);
> if (prp != NULL) {
> /*
> --
> 2.18.4
>