Em Fri, Nov 30, 2012 at 02:29:43PM +0100, Jiri Olsa escreveu:
> On Thu, Nov 29, 2012 at 03:38:40PM +0900, Namhyung Kim wrote:
> > +#define __HPP_COLOR_PERCENT_FN(_type, _field)                              
> >         \
> > +static int hpp__color_##_type(struct perf_hpp *hpp, struct hist_entry *he) 
> >         \
> > +{                                                                          
> > \
> > +   int ret;                                                                
> > \
> > +   double percent = 0.0;                                                   
> > \
> > +   struct hists *hists = he->hists;                                        
> > \
> > +                                                                           
> > \
> > +   if (hists->stats.total_period)                                          
> > \
> > +           percent = 100.0 * he->stat._field / hists->stats.total_period;  
> > \
> > +                                                                           
> > \
> > +   ret = percent_color_snprintf(hpp->buf, hpp->size, " %6.2f%%", percent); 
> > \
> > +                                                                           
> > \
> > +   if (symbol_conf.event_group) {                                          
> > \
> > +           int i;                                                          
> > \
> > +           struct perf_evsel *evsel = hists_to_evsel(hists);               
> > \
> > +           struct hist_entry *pair;                                        
> > \
> > +           int nr_members = evsel->nr_members;                             
> > \
> > +           double *percents;                                               
> > \
> > +                                                                           
> > \
> > +           if (nr_members <= 1)                                            
> > \
> > +                   return ret;                                             
> > \
> > +                                                                           
> > \
> > +           percents = zalloc(sizeof(*percents) * nr_members);              
> > \
> > +           if (percents == NULL) {                                         
> > \
> > +                   pr_warning("Not enough memory!\n");                     
> > \
> > +                   return ret;                                             
> > \
> > +           }                                                               
> > \

Why do we need to zalloc this percents array?

> > +           list_for_each_entry(pair, &he->pairs.head, pairs.node) {        
> > \
> > +                   u64 period = pair->stat._field;                         
> > \
> > +                   u64 total = pair->hists->stats.total_period;            
> > \
> > +                                                                           
> > \
> > +                   if (!total)                                             
> > \
> > +                           continue;                                       
> > \
> > +                                                                           
> > \
> > +                   evsel = hists_to_evsel(pair->hists);                    
> > \
> > +                   i = perf_evsel__group_idx(evsel);                       
> > \
> > +                   percents[i] = 100.0 * period / total;                   
> > \

Why not use percent_color_snprintf here, using some "%*s" thing that
uses i to position the output in the right column? This way the
following loop can be ditched as well. No?

> > +           }                                                               
> > \
> > +                                                                           
> > \
> > +           for (i = 1; i < nr_members; i++) {                              
> > \
> > +                   ret += percent_color_snprintf(hpp->buf + ret,           
> > \
> > +                                                 hpp->size - ret,          
> > \
> > +                                                 " %6.2f%%", percents[i]); 
> > \
> > +           }                                                               
> > \
> > +           free(percents);                                                 
> > \
> > +   }                                                                       
> > \
> > +   return ret;                                                             
> > \

- Arnaldo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to