[DTrace-devel] [PATCH 1/2] Minor code cleanup

[Kris Van Hees]

On Tue, Aug 11, 2020 at 03:28:11PM -0700, Eugene Loh wrote:
> Review please.
> 
> 
> On 07/23/2020 02:07 PM, eugene.loh at oracle.com wrote:
> > From: Eugene Loh <eugene.loh at oracle.com>
> >
> > Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
> > ---
> >   libdtrace/dt_printf.c | 14 ++++++--------
> >   1 file changed, 6 insertions(+), 8 deletions(-)
> >
> > diff --git a/libdtrace/dt_printf.c b/libdtrace/dt_printf.c
> > index 03ffa8bd..4382c4bf 100644
> > --- a/libdtrace/dt_printf.c
> > +++ b/libdtrace/dt_printf.c
> > @@ -1549,7 +1549,7 @@ dtrace_freopen(dtrace_hdl_t *dtp, FILE *fp, void *fmtdata,
> >       const dtrace_probedata_t *data, const dtrace_recdesc_t *recp,
> >       uint_t nrecs, const void *buf, size_t len)
> >   {
> > -	char selfbuf[40], restorebuf[40];
> > +	char tmpbuf[40];

OK

> >   	FILE *nfp;
> >   	int rval, errval;
> >   	dt_pfargv_t *pfv = fmtdata;
> > @@ -1580,11 +1580,10 @@ dtrace_freopen(dtrace_hdl_t *dtp, FILE *fp, void *fmtdata,
> >   			return (rval);
> >   		}
> >   
> > -		(void) snprintf(restorebuf, sizeof (restorebuf),
> > +		(void) snprintf(tmpbuf, sizeof (tmpbuf),
> >   		    "/dev/fd/%d", dtp->dt_stdout_fd);

Drop the "(void) " since that isn't necessary.

> >   		free(dtp->dt_freopen_filename);
> > -		dtp->dt_freopen_filename = strndup(restorebuf,
> > -						   sizeof(restorebuf));
> > +		dtp->dt_freopen_filename = strndup(tmpbuf, sizeof(tmpbuf));

OK

> >   	} else {
> >   		free(dtp->dt_freopen_filename);
> >   		dtp->dt_freopen_filename = strdup(dtp->dt_sprintf_buf);
> > @@ -1593,8 +1592,7 @@ dtrace_freopen(dtrace_hdl_t *dtp, FILE *fp, void *fmtdata,
> >   	/*
> >   	 * freopen(3C) will always close the specified stream and underlying
> >   	 * file descriptor -- even if the specified file can't be opened.
> > -	 * Even for the semantic cesspool that is standard I/O, this is
> > -	 * surprisingly brain-dead behavior:  it means that any failure to
> > +	 * This means that any failure to

While I agree that the original authors could have been a bit more polite in
their phrasing, I don't think changing it as part of this patch makes sense.
If we want to sanitize the comments in the code, we should do so in patches
that are specifically written for that.

> >   	 * open the specified file destroys the specified stream in the
> >   	 * process -- which is particularly relevant when the specified stream
> >   	 * happens (or rather, happened) to be stdout.  This could be resolved
> > @@ -1620,7 +1618,7 @@ dtrace_freopen(dtrace_hdl_t *dtp, FILE *fp, void *fmtdata,
> >   		return (errval);
> >   	}
> >   
> > -	(void) snprintf(selfbuf, sizeof (selfbuf), "/dev/fd/%d", fileno(nfp));
> > +	(void) snprintf(tmpbuf, sizeof (tmpbuf), "/dev/fd/%d", fileno(nfp));

Drop the "(void) " since that isn't necessary.

> >   
> >   	if (dtp->dt_stdout_fd == -1) {
> >   		/*
> > @@ -1635,7 +1633,7 @@ dtrace_freopen(dtrace_hdl_t *dtp, FILE *fp, void *fmtdata,
> >   		}
> >   	}
> >   
> > -	if (freopen(selfbuf, "aF", fp) == NULL) {
> > +	if (freopen(tmpbuf, "aF", fp) == NULL) {
> >   		(void) fclose(nfp);
> >   		return (dt_set_errno(dtp, errno));
> >   	}
> 
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel