Re: [DTrace-devel] [PATCH v7 2/7] support stapsdt ELF-note-defined static probes

On Tue, Aug 05, 2025 at 02:49:52PM -0400, Kris Van Hees wrote:
> Comments below.

I wrote too hastily... see below...
> 
> On Wed, Jul 30, 2025 at 10:01:43AM +0100, Alan Maguire via DTrace-devel wrote:
<< skip >>
> > +static int
> > +dt_pid_create_stapsdt_probes(dtrace_probedesc_t *pdp, dtrace_hdl_t *dtp, dt_pcb_t *pcb)
> > +{
> > +	const dt_provider_t *pvp;
> > +	dt_proc_t *dpr = NULL;
> > +	const char *pidstr;
> > +	char *path = NULL;
> > +	pid_t pid;
> > +
> > +	assert(pcb != NULL);
> > +
> > +	pidstr = &pdp->prv[strlen(pdp->prv)];
> > +
> > +	while (isdigit(*(pidstr - 1)))
> > +		pidstr--;
> > +	if (strlen(pidstr) == 0)
> > +		return 0;
> > +
> > +	asprintf(&path, "/proc/%s/maps", pidstr);
> 
> If this fails, asprintf() returns -1, and path is undefined.  So there should
> be a check to deal with that.

Actually, since path is only used in the code block near the end of the funtion
there is no point initializing path here, because that creates multiple places
where you would need to free it.  Just move the declaration of path and the
asprintf into the code block.
> 
> > +
> > +	pvp = dt_provider_lookup(dtp, "stapsdt");
> > +	assert(pvp != NULL);
> > +
> > +	pid = atoll(pidstr);
> > +	if (pid <= 0)
> > +		return 0;
> > +	if (dt_proc_grab_lock(dtp, pid, DTRACE_PROC_WAITING |
> > +			      DTRACE_PROC_SHORTLIVED) < 0) {
> > +		dt_pid_error(dtp, pcb, NULL, D_PROC_GRAB,
> > +			     "failed to grab process %d",
> > +			     (int)pid);
> > +		return 1;
> > +	}
> > +	dpr = dt_proc_lookup(dtp, pid);
> > +	if (dpr) {

char *path = NULL;

if (asprintf(...) == -1)
  longjmp(pcb->pcb_jumpbuf, EDT_NOMEM);

> > +		dt_pid_create_stapsdt_probes_proc(pdp, dtp, pcb,
> > +						  pvp, dpr, path);

free(path)

> > +		dt_proc_release_unlock(dtp, pid);
> > +	}
> > +
> > +	return 0;
> > +}