Re: [DTrace-devel] [PATCH v5 07/19] Create the BPF usdt_names and usdt_prids maps

[Kris Van Hees <kris.van.hees@oracle.com>]

On Sat, Oct 12, 2024 at 12:04:59AM -0400, Eugene Loh wrote:
> On 10/11/24 19:53, Kris Van Hees wrote:
> 
> > On Fri, Oct 11, 2024 at 06:56:16PM -0400, Eugene Loh via DTrace-devel wrote:
> > > On 10/10/24 00:55, Kris Van Hees wrote:
> > > > On Wed, Oct 09, 2024 at 01:45:34AM -0400, eugene.loh@oracle.com wrote:
> > > > > From: Eugene Loh <eugene.loh@oracle.com>
> > > > > +	if (dtp->dt_usdt_namesmap_fd == -1)
> > > > > +		return -1;
> > > > > +
> > > > > +	dtp->dt_usdt_pridsmap_fd = create_gmap(dtp, "usdt_prids", BPF_MAP_TYPE_HASH,
> > > > > +	    dtp->dt_usdt_pridsmap_ksz, dtp->dt_usdt_pridsmap_vsz,
> > > > > +	    dtp->dt_options[DTRACEOPT_NUSDTPROBES]);
> > > > empty line perhaps?0~
> > > Sorry, I don't understand this comment.
> > Similar to the previous assignment followed by a conditional, I think that it
> > would make sense to insert an empty line between the assignment and the
> > conditional.
> 
> Curious.  It seemed to me to be very common for us to assign something and
> then immediately (no blank line) check its validity. Just a quick sanity
> check (not using this file, which at this point I've touched extensively), I
> tried libdtrace/dt_bpf.c.  Indeed, lots of this practice, and you seemed to
> be the last person to touch those spots.  I like the close spacing, but
> don't care a whole lot; I just wanted to double check since the patch seems
> to follow common practice in this particular regard.

Ah, oops, ignore me.  That was a comment I originally wrote, then meant to
delete (note the odd characters following it in my original review email), but
clearly botched that.  And then when you asked about it, I interpreted my
comment rather than actually looking back at what code it was really refering
to.

My mistake - please ignore.  Sorry for the confusion.