On Fri, 15 Jun 2018 at 15:59, Kim Phillips kim.phillips@arm.com wrote:
On Fri, 15 Jun 2018 15:41:52 -0600 Mathieu Poirier mathieu.poirier@linaro.org wrote:
On Fri, 15 Jun 2018 at 15:06, Kim Phillips kim.phillips@arm.com wrote:
On Fri, 15 Jun 2018 10:03:43 -0600 Mathieu Poirier mathieu.poirier@linaro.org wrote:
On Thu, 14 Jun 2018 at 17:53, Kim Phillips kim.phillips@arm.com wrote:
On Thu, 14 Jun 2018 13:41:33 -0600 Mathieu Poirier mathieu.poirier@linaro.org wrote:
@@ -430,24 +420,30 @@ static int cs_etm__setup_queue(struct cs_etm_auxtrace *etm, struct auxtrace_queue *queue, unsigned int queue_nr) {
int ret = 0; struct cs_etm_queue *etmq = queue->priv; if (list_empty(&queue->head) || etmq)
return 0;
goto out;What does 'goto out' buy us here? Having the return in-place is easier/clearer to read.
It streamlines the exit path.
How? It unnecessarily redirects it and sends the reader hunting.
This is how the function body looks after this patch:
{ int ret = 0; struct cs_etm_queue *etmq = queue->priv;
if (list_empty(&queue->head) || etmq) goto out; etmq = cs_etm__alloc_queue(etm); if (!etmq) { ret = -ENOMEM; goto out; } queue->priv = etmq; etmq->etm = etm; etmq->queue_nr = queue_nr; etmq->cpu = queue->cpu; etmq->tid = queue->tid; etmq->pid = -1; etmq->offset = 0; etmq->period_instructions = 0;out: return ret; }
The real gain from this approach becomes clearer when timestamp initialisation (second patchset) gets thrown in the mix.
It should be introduced there then.
I do not agree. The goal of the patch is to re-write the alloc/setup function, which is exactly what I'm doing.
Thanks,
Kim