[DTrace-devel] [PATCH 6/6] Disassembler: coding-style taste test

[Kris Van Hees]

Let's not do this.  I don't think there is any real gain from making those
two functions one-liners based on a larger macro.

On Fri, Mar 19, 2021 at 01:45:34PM -0400, eugene.loh at oracle.com wrote:
> From: Eugene Loh <eugene.loh at oracle.com>
> 
> The previous patch for the disassembler replaces
>     dt_dis_varname()
> with
>     dt_dis_varname_id() and
>     dt_dis_varname_off()
> 
> Since the two new functions are very similar, this patch proposes
> a preprocessor macro that essentially implements both, making each
> function basically a one-liner.  Is this an improvement?  I don't
> know.  Take a look.
> 
> This patch should either be discarded or squashed into the earlier
> disassembler patch.  The earlier commit message could be used without
> change.
> 
> Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
> ---
>  libdtrace/dt_dis.c | 38 ++++++++++++++++----------------------
>  1 file changed, 16 insertions(+), 22 deletions(-)
> 
> diff --git a/libdtrace/dt_dis.c b/libdtrace/dt_dis.c
> index 1a589bca..5505b339 100644
> --- a/libdtrace/dt_dis.c
> +++ b/libdtrace/dt_dis.c
> @@ -44,23 +44,27 @@ reg(int r)
>  	return name[r];
>  }
>  
> +#define DT_DIS_VARNAME_LOOKUP(field, x) \
> +	for (i = 0; i < dp->dtdo_varlen; i++) { \
> +		const dtrace_difv_t *dvp = &dp->dtdo_vartab[i]; \
> +		if (dvp->field == (x) && \
> +		    dvp->dtdv_scope == scope && \
> +		    dvp->dtdv_insn_from <= addr && \
> +		    dvp->dtdv_insn_to >= addr) { \
> +			if (dvp->dtdv_name < dp->dtdo_strlen) \
> +				return dp->dtdo_strtab + dvp->dtdv_name; \
> +			break; \
> +		} \
> +	} \
> +	return NULL
> +
>  static const char *
>  dt_dis_varname_id(const dtrace_difo_t *dp, uint_t id, uint_t scope, uint_t addr)
>  {
> -	const dtrace_difv_t *dvp = dp->dtdo_vartab;
>  	uint_t i;
>  
>  	id &= 0x0fff;
> -	for (i = 0; i < dp->dtdo_varlen; i++, dvp++) {
> -		if (dvp->dtdv_id == id && dvp->dtdv_scope == scope &&
> -		    dvp->dtdv_insn_from <= addr && addr <= dvp->dtdv_insn_to) {
> -			if (dvp->dtdv_name < dp->dtdo_strlen)
> -				return dp->dtdo_strtab + dvp->dtdv_name;
> -			break;
> -		}
> -	}
> -
> -	return NULL;
> +	DT_DIS_VARNAME_LOOKUP(dtdv_id, id); /* look up by id */
>  }
>  
>  static const char *
> @@ -68,17 +72,7 @@ dt_dis_varname_off(const dtrace_difo_t *dp, uint_t off, uint_t scope, uint_t add
>  {
>  	uint_t i;
>  
> -	for (i = 0; i < dp->dtdo_varlen; i++) {
> -		const dtrace_difv_t *dvp = &dp->dtdo_vartab[i];
> -		if (dvp->dtdv_offset == off && dvp->dtdv_scope == scope &&
> -		    dvp->dtdv_insn_from <= addr && addr <= dvp->dtdv_insn_to) {
> -			if (dvp->dtdv_name < dp->dtdo_strlen)
> -				return dp->dtdo_strtab + dvp->dtdv_name;
> -			break;
> -		}
> -	}
> -
> -	return NULL;
> +	DT_DIS_VARNAME_LOOKUP(dtdv_offset, off); /* look up by offset */
>  }
>  
>  /*ARGSUSED*/
> -- 
> 2.18.4
> 
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel