From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1751262AbdBHUZL (ORCPT <rfc822;w@1wt.eu>);
        Wed, 8 Feb 2017 15:25:11 -0500
Received: from mga03.intel.com ([134.134.136.65]:54028 "EHLO mga03.intel.com"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1750940AbdBHUZK (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
        Wed, 8 Feb 2017 15:25:10 -0500
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.35,348,1484035200"; 
   d="scan'208";a="63453428"
Message-ID: <1486585508.2871.11.camel@tzanussi-mobl.amr.corp.intel.com>
Subject: Re: [RFC][PATCH 02/21] tracing: Reimplement log2
From: Tom Zanussi <tom.zanussi@linux.intel.com>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: tglx@linutronix.de, mhiramat@kernel.org, namhyung@kernel.org,
        linux-kernel@vger.kernel.org, linux-rt-users@vger.kernel.org
Date: Wed, 08 Feb 2017 14:25:08 -0600
In-Reply-To: <20170208151358.560a2c64@gandalf.local.home>
References: <cover.1486569306.git.tom.zanussi@linux.intel.com>
         <b247aa8251fe3d92695d63ec974d82f80ee4e711.1486569306.git.tom.zanussi@linux.intel.com>
         <20170208151358.560a2c64@gandalf.local.home>
Content-Type: text/plain; charset="UTF-8"
X-Mailer: Evolution 3.10.4 (3.10.4-4.fc20) 
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Wed, 2017-02-08 at 15:13 -0500, Steven Rostedt wrote:
> On Wed,  8 Feb 2017 11:24:58 -0600
> Tom Zanussi <tom.zanussi@linux.intel.com> wrote:
> 
> >  static void destroy_hist_field(struct hist_field *hist_field)
> >  {
> > +	unsigned int i;
> > +
> > +	if (!hist_field)
> > +		return;
> > +
> > +	for (i = 0; i < HIST_FIELD_OPERANDS_MAX; i++)
> > +		destroy_hist_field(hist_field->operands[i]);
> 
> Recursive functions get me really nervous. What limits it? Is this user
> defined? Perhaps we need to find a better way to handle this that's not
> recursive, or at least put in a hard limit of the amount it can recurse.
> 

It's limited by the expression depth, which shouldn't be more than 1
deep, but you're right, there should be an explicit limit check, just in
case - will add one.

Tom

> -- Steve
> 
> > +
> >  	kfree(hist_field);
> >  }
> >  
> > @@ -377,7 +393,10 @@ static struct hist_field *create_hist_field(struct ftrace_event_field *field,
> >  	}
> >  
> >  	if (flags & HIST_FIELD_FL_LOG2) {
> > +		unsigned long fl = flags & ~HIST_FIELD_FL_LOG2;
> >  		hist_field->fn = hist_field_log2;
> > +		hist_field->operands[0] = create_hist_field(field, fl);
> > +		hist_field->size = hist_field->operands[0]->size;
> >  		goto out;
> >  	}
> >  
>