Messages in this thread | | | Date | Sun, 29 Jun 2014 10:50:24 -0600 | From | David Ahern <> | Subject | Re: [PATCH 05/18] perf tools: Add ordered_events_(get|put) interface |
| |
On 6/29/14, 10:39 AM, Jiri Olsa wrote: > On Fri, Jun 27, 2014 at 05:06:36PM -0600, David Ahern wrote: >> On 6/18/14, 8:58 AM, Jiri Olsa wrote: >>> >>> +static struct ordered_event* >>> +ordered_events_get(struct ordered_events_queue *q, u64 timestamp) >>> +{ >>> + struct ordered_event *new; >>> + >>> + new = alloc_event(q); >>> + if (new) { >>> + new->timestamp = timestamp; >>> + queue_event(q, new); >>> + } >>> + >>> + return new; >>> +} >> >> The _get name does not really correlate with what is happening -- ie., >> allocate a new event and add it to the queue. There is no reference taken >> either. > > ook.. so how about ordered_events_alloc ordered_events_free > >> >>> + >>> +static void >>> +ordered_event_put(struct ordered_events_queue *q, struct ordered_event *iter) >>> +{ >>> + list_del(&iter->list); >>> + list_add(&iter->list, &q->cache); >>> + q->nr_events--; >>> +} >> >> Similarly here with the _put. In this case the function is moving the event >> from one list to another. And how about something else for the name besides >> iter -- oe, or oevent? > > how about 'event' ?
Already a heavily used keyword in perf, that's why I was thinking oe or oevent -- besides it is a struct ordered_event not an event.
The bigger thing to me with this patch is the _get/_put names.
David
| |