On Sat, Dec 20, 2014 at 03:11:54PM +0100, Frank Schäfer wrote:
> Hi Russel,

I guess you won't mind if I mis-spell your name too...

> I'd prefer to keep the button initialization related stuff together in
> em28xx_init_buttons() and do the cancel_delayed_work_sync() only if we
> have buttons (dev->num_button_polling_addresses).
> That's how we already do it with the IR work struct (see
> em28xx_ir_suspend()).

Provided all places that touch buttons_query_work are properly updated
that's fine, but to me that is fragile and asking for trouble.  It's far
better to ensure that everything is properly initialised so you don't
have to remember to conditionalise every single reference to a work
struct.

In any case, delayed work struct initialisation is cheap - it doesn't
involve any additional memory, it only initialises the various members
of the struct (and the lockdep information for the static key) so there
really is no argument against always initialising delayed works or normal
works, timers, etc to avoid these kinds of bugs.

Anything to keep the code simple is a good thing.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to