Re: [PATCH RFC 3/7] ALSA: hda - Make snd_hda_jack_detect_enable_callbac() returning the jack object

[David Henningsson <david.henningsson@canonical.com>]

Nitpick: s/callbac/callback and s/returning/return in subject

On 2014-09-11 16:19, Takashi Iwai wrote:
> STAC/IDT driver calls snd_hda_jack_tbl_get() again after calling
> snd_hda_jack_detect_enable_callback().  For simplifying this, let's
> make snd_hda_jack_detect_enable_callback() returning the pointer while
> handling the error with the standard IS_ERR() & co.
>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>   sound/pci/hda/hda_jack.c       | 20 +++++++++++++-------
>   sound/pci/hda/hda_jack.h       |  5 +++--
>   sound/pci/hda/patch_sigmatel.c | 14 ++++++--------
>   3 files changed, 22 insertions(+), 17 deletions(-)
>
> diff --git a/sound/pci/hda/hda_jack.c b/sound/pci/hda/hda_jack.c
> index 7f332794993f..27fccd2c8d41 100644
> --- a/sound/pci/hda/hda_jack.c
> +++ b/sound/pci/hda/hda_jack.c
> @@ -215,28 +215,34 @@ EXPORT_SYMBOL_GPL(snd_hda_jack_detect_state);
>   /**
>    * snd_hda_jack_detect_enable - enable the jack-detection
>    */
> -int snd_hda_jack_detect_enable_callback(struct hda_codec *codec, hda_nid_t nid,
> -					hda_jack_callback cb)
> +struct hda_jack_tbl *
> +snd_hda_jack_detect_enable_callback(struct hda_codec *codec, hda_nid_t nid,
> +				    hda_jack_callback cb)

I have not seen the ERR_PTR usage before and it's not used commonly in 
the hda driver, so I think it's important to document that it can return 
a non-null pointer that is not a valid hda_jack_tbl. Looks dangerous 
otherwise.

-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic