Re: [PATCH 15/16] ehca: queue page table handling

[Jörn Engel]

On Thu, 27 April 2006 12:49:50 +0200, Heiko J Schick wrote:
> +inline static void *ipz_qeit_get_inc_valid(struct ipz_queue *queue)
> +{
> +	void *retvalue = ipz_qeit_get(queue);
> +	u32 qe = ((struct ehca_cqe *)retvalue)->cqe_flags;
> +	if ((qe >> 7) == (queue->toggle_state & 1)) {
> +		/* this is a good one */
> +		ipz_qeit_get_inc(queue);
> +	} else
> +		retvalue = NULL;
> +	return (retvalue);
> +}

How about:
static inline void *ipz_qeit_get_inc_valid(struct ipz_queue *queue)
{
	struct ehca_cqe *cqe = ipz_qeit_get(queue);
	u32 flags = cqe->cqe_flags;

	if ((flags >> 7) != (queue->toggle_state & 1))
		return NULL;

	ipz_qeit_get_inc(queue);
	return cqe;
}

o "static inline", as Arnd requested,
o no cast for cqe,
o possibly useful identifier for "retvalue",
o trivial to identify error path (hint: only error path is indented),
o directly returns NULL instead of assigning to a variable,
o removed brackets around return value.

I'm still not happy with "ehca_cqe" (just try to pronounce it) and the
weird condition.  But you should get the general idea.  Same goes for
other functions.

Jörn

-- 
The cheapest, fastest and most reliable components of a computer
system are those that aren't there.
-- Gordon Bell, DEC labratories
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/