[bug report] ALSA: hda - Don't register a cb func if it is registered already

[Hui Wang]

On 10/21/20 8:19 PM, Dan Carpenter wrote:
> Hello Hui Wang,
>
> The patch f4794c6064a8: "ALSA: hda - Don't register a cb func if it
> is registered already" from Sep 30, 2020, leads to the following
> static checker warning:
>
> 	sound/pci/hda/patch_sigmatel.c:3075 stac92hd71bxx_fixup_hp_m4()
> 	warn: 'jack' can also be NULL
>
> sound/pci/hda/patch_sigmatel.c
>    3069          /* Enable VREF power saving on GPIO1 detect */
>    3070          snd_hda_codec_write_cache(codec, codec->core.afg, 0,
>    3071                                    AC_VERB_SET_GPIO_UNSOLICITED_RSP_MASK, 0x02);
>    3072          jack = snd_hda_jack_detect_enable_callback(codec, codec->core.afg,
>    3073                                                     stac_vref_event);
>
> Originally snd_hda_jack_detect_enable_callback() would not return NULL
> here.
>
>    3074          if (!IS_ERR(jack))
>    3075                  jack->private_data = 0x02;
>    3076
>    3077          spec->gpio_mask |= 0x02;
>
> But now we have this:
>
> sound/pci/hda/hda_jack.c
>     301  struct hda_jack_callback *
>     302  snd_hda_jack_detect_enable_callback_mst(struct hda_codec *codec, hda_nid_t nid,
>     303                                          int dev_id, hda_jack_callback_fn func)
>     304  {
>     305          struct hda_jack_tbl *jack;
>     306          struct hda_jack_callback *callback = NULL;
>     307          int err;
>     308
>     309          jack = snd_hda_jack_tbl_new(codec, nid, dev_id);
>     310          if (!jack)
>     311                  return ERR_PTR(-ENOMEM);
>     312          if (func && !func_is_already_in_callback_list(jack, func)) {
>                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> We only allocate callback if there isn't one already.
Yes, indeed, this is a problem.
>
>     313                  callback = kzalloc(sizeof(*callback), GFP_KERNEL);
>     314                  if (!callback)
>     315                          return ERR_PTR(-ENOMEM);
>     316                  callback->func = func;
>     317                  callback->nid = jack->nid;
>     318                  callback->dev_id = jack->dev_id;
>     319                  callback->next = jack->callback;
>     320                  jack->callback = callback;
>     321          }
>     322
>     323          if (jack->jack_detect)
>     324                  return callback; /* already registered */
>                          ^^^^^^^^^^^^^^^
> So presumably this should be jack->callback

Looks like it is also not correct to return the jack->callback.  Need to 
take some time to write a fix for it.


Thanks for reporting the issue.

>
>
>     325          jack->jack_detect = 1;
>     326          if (codec->jackpoll_interval > 0)
>     327                  return callback; /* No unsol if we're polling instead */
>                          ^^^^^^^^^^^^^^^^
>
>     328          err = snd_hda_codec_write_cache(codec, nid, 0,
>     329                                           AC_VERB_SET_UNSOLICITED_ENABLE,
>     330                                           AC_USRSP_EN | jack->tag);
>     331          if (err < 0)
>     332                  return ERR_PTR(err);
>     333          return callback;
>                  ^^^^^^^^^^^^^^^^
> And these as well.
>
>     334  }
>
> regards,
> dan carpenter