On 06/03/18 22:25, Arnaldo Carvalho de Melo wrote:
> Em Tue, Mar 06, 2018 at 11:13:17AM +0200, Adrian Hunter escreveu:
>> In preparation for supporting AUX area sampling buffers,
>> auxtrace_queues__add_buffer() needs to be more generic. To that end, move
>> memory allocation for struct buffer into it.
>>
>> Signed-off-by: Adrian Hunter <adrian.hun...@intel.com>
>> ---
>> tools/perf/util/auxtrace.c | 54
>> +++++++++++++++++++++-------------------------
>> 1 file changed, 24 insertions(+), 30 deletions(-)
>>
>> diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c
>> index fb357a00dd86..e1aff91c54a8 100644
>> --- a/tools/perf/util/auxtrace.c
>> +++ b/tools/perf/util/auxtrace.c
>> @@ -308,7 +308,11 @@ static int auxtrace_queues__add_buffer(struct
>> auxtrace_queues *queues,
>> struct auxtrace_buffer *buffer,
>> struct auxtrace_buffer **buffer_ptr)
>> {
>> - int err;
>> + int err = -ENOMEM;
>> +
>> + buffer = memdup(buffer, sizeof(*buffer));
>
> this is a bit strange, why not make buffer a local variable in this
> function then?
Do you mean the following?
struct auxtrace_buffer *new_buf;
new_buf = memdup(buffer, sizeof(*buffer));
>
>> + if (!buffer)
>> + return -ENOMEM;
>>
>> if (session->one_mmap) {
>> buffer->data = buffer->data_offset - session->one_mmap_offset +
>> @@ -316,24 +320,28 @@ static int auxtrace_queues__add_buffer(struct
>> auxtrace_queues *queues,
>> } else if (perf_data__is_pipe(session->data)) {
>> buffer->data = auxtrace_copy_data(buffer->size, session);
>> if (!buffer->data)
>> - return -ENOMEM;
>> + goto out_free;
>> buffer->data_needs_freeing = true;
>> } else if (BITS_PER_LONG == 32 &&
>> buffer->size > BUFFER_LIMIT_FOR_32_BIT) {
>> err = auxtrace_queues__split_buffer(queues, idx, buffer);
>> if (err)
>> - return err;
>> + goto out_free;
>> }
>>
>> err = auxtrace_queues__queue_buffer(queues, idx, buffer);
>> if (err)
>> - return err;
>> + goto out_free;
>>
>> /* FIXME: Doesn't work for split buffer */
>> if (buffer_ptr)
>> *buffer_ptr = buffer;
>>
>> return 0;
>> +
>> +out_free:
>> + auxtrace_buffer__free(buffer);
>> + return err;
>> }
>>
>> static bool filter_cpu(struct perf_session *session, int cpu)
>> @@ -348,36 +356,22 @@ int auxtrace_queues__add_event(struct auxtrace_queues
>> *queues,
>> union perf_event *event, off_t data_offset,
>> struct auxtrace_buffer **buffer_ptr)
>> {
>> - struct auxtrace_buffer *buffer;
>> - unsigned int idx;
>> - int err;
>> + struct auxtrace_buffer buffer = {
>> + .pid = -1,
>> + .tid = event->auxtrace.tid,
>> + .cpu = event->auxtrace.cpu,
>> + .data_offset = data_offset,
>> + .offset = event->auxtrace.offset,
>> + .reference = event->auxtrace.reference,
>> + .size = event->auxtrace.size,
>> + };
>> + unsigned int idx = event->auxtrace.idx;
>>
>> if (filter_cpu(session, event->auxtrace.cpu))
>> return 0;
>>
>> - buffer = zalloc(sizeof(struct auxtrace_buffer));
>> - if (!buffer)
>> - return -ENOMEM;
>> -
>> - buffer->pid = -1;
>> - buffer->tid = event->auxtrace.tid;
>> - buffer->cpu = event->auxtrace.cpu;
>> - buffer->data_offset = data_offset;
>> - buffer->offset = event->auxtrace.offset;
>> - buffer->reference = event->auxtrace.reference;
>> - buffer->size = event->auxtrace.size;
>> - idx = event->auxtrace.idx;
>> -
>> - err = auxtrace_queues__add_buffer(queues, session, idx, buffer,
>> - buffer_ptr);
>> - if (err)
>> - goto out_err;
>> -
>> - return 0;
>> -
>> -out_err:
>> - auxtrace_buffer__free(buffer);
>> - return err;
>> + return auxtrace_queues__add_buffer(queues, session, idx, &buffer,
>> + buffer_ptr);
>> }
>>
>> static int auxtrace_queues__add_indexed_event(struct auxtrace_queues
>> *queues,
>> --
>> 1.9.1
>
