[PATCH] Staging: d53155_drv.c: cleanup fbuffer usage

Joe Perches joe at perches.com
Thu Jun 24 17:37:32 UTC 2010


On Thu, 2010-06-24 at 09:31 -0700, H Hartley Sweeten wrote:
> The global symbol dt3155_fbuffer[], declared in dt3155_isr.c, is really
> just a pointer to dt3155_status[].fbuffer. To improve readability, make
> some of the really long lines shorter, and make the buffer access more
> consistent, use &dt3155_status[].fbuffer to access the buffer structure.

I think this would be more consistent/intelligible
if the temporary wasn't
	struct dt3155_fbuffer *fb = &dt3155_status[minor].fbuffer;
but was
	struct dt3155_status *dts = &dt3155_status[minor];

> @@ -146,11 +148,11 @@ static void quick_stop (int minor)
>    dt3155_status[minor].state &= ~(DT3155_STATE_STOP|0xff);
>    /* mark the system stopped: */
>    dt3155_status[minor].state |= DT3155_STATE_IDLE;
> -  dt3155_fbuffer[minor]->stop_acquire = 0;
> -  dt3155_fbuffer[minor]->even_stopped = 0;
> +  fb->stop_acquire = 0;
> +  fb->even_stopped = 0;
>  #else
>    dt3155_status[minor].state |= DT3155_STATE_STOP;
> -  dt3155_status[minor].fbuffer.stop_acquire = 1;
> +  fb->stop_acquire = 1;
>  #endif

becomes

  dts->state &= ~(DT3155_STATE_STOP|0xff);
  /* mark the system stopped: */
  dts->state |= DT3155_STATE_IDLE;
  dts->fbuffer.stop_acquire = 0;
  dts->fbuffer.even_stopped = 0;
#else
  dts->fbuffer.stop_acquire = 1;
#endif

Another option might be to use dereferencing inlines.

static inline struct dt3155_status *dts(int index)
{
	return &dt3155_status[index];
}
static inline struct dt3155_fbuffer *fb(int index)
{
	return &(dts(index)->fbuffer);
}

I think this isn't as good as the temporary because the
compiler sometimes doesn't reuse the intermediate addresses
and the inlines have file rather than function scope.

It becomes something like:

  dts(minor)->state &= ~(DT3155_STATE_STOP|0xff);
  /* mark the system stopped: */
  dts(minor)->state |= DT3155_STATE_IDLE;
  fb(minor)->stop_acquire = 0;
  fb(minor)->even_stopped = 0;
#else
  fb(minor)->stop_acquire = 1;
#endif

I think this isn't as good as the temporary because the
compiler sometimes doesn't reuse the intermediate addresses
and the macros have file rather than function scope.





More information about the devel mailing list