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.
etmq = cs_etm__alloc_queue(etm, queue_nr);
etmq = cs_etm__alloc_queue(etm);
if (!etmq)
return -ENOMEM;
if (!etmq) {
ret = -ENOMEM;
goto out;
}
same here.
queue->priv = etmq;
if (queue->cpu != -1)
etmq->cpu = queue->cpu;
etmq->etm = etm;
etmq->queue_nr = queue_nr;
etmq->cpu = queue->cpu;
the != -1 condition above is being removed and not replaced, yet there's no explanation in the commit text. Should it be a part of a different patch?
Variable queue->cpu is either -1 (per-thread) or hold the value of a CPU (CPU-wide). So instead of invariably setting etmq->cpu to -1 in cs_etm__alloc_queue() and checking if it is worth changing that in cs_etm__setup_queue() as the current code does, directly doing etmq->cpu = queue->cpu yields exactly the same result.
Kim