On Thu, Mar 1, 2018 at 9:13 PM, Rodrigo Rivas Costa <rodrigorivasco...@gmail.com> wrote: > On Thu, Mar 01, 2018 at 12:20:37PM +0200, Andy Shevchenko wrote: >> On Thu, Mar 1, 2018 at 12:49 AM, Rodrigo Rivas Costa >> <rodrigorivasco...@gmail.com> wrote: >> > On Wed, Feb 28, 2018 at 09:21:15PM +0200, Andy Shevchenko wrote: >> >> On Wed, Feb 28, 2018 at 8:43 PM, Rodrigo Rivas Costa >> >> <rodrigorivasco...@gmail.com> wrote:
> My bad, I didn't mean a literal free() call. More like: > > static void steam_unregister(struct steam_device *steam) > { > struct input_dev *input; > struct power_supply *battery; > > rcu_read_lock(); > input = rcu_dereference(steam->input); > battery = rcu_dereference(steam->battery); > input_gyro = rcu_dereference(steam->input_gyro); > rcu_read_unlock(); > > if (input) { > RCU_INIT_POINTER(steam->input, NULL); > synchronize_rcu(); > hid_info(steam->hdev, "Steam Controller '%s' disconnected", > steam->serial_no); > input_unregister_device(input); Candidate for a helper static void steam_unregister_input(...) { ... } steam_unregister_input(input); > } > if (battery) { > RCU_INIT_POINTER(steam->battery, NULL); > synchronize_rcu(); > power_supply_unregister(battery); > } Ditto. > if (input_gyro) { > RCU_INIT_POINTER(steam->input_gyro, NULL); > synchronize_rcu(); > input_unregister_device(input_gyro); > } Ditto. > } > > Also I think input_unregister_device() does not admit a NULL pointer. > Having a special `if (!input_gyro)return;` at the end looks just > wrong to me, it is no different from the other ones. See above. Hide that detail in a helper function. -- With Best Regards, Andy Shevchenko