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

[Hui Wang]

Looks like this will not bring problem on patch_sigmatel.c, NULL and 
valid kernel pointer are same for IS_ERR(), they will not make IS_ERR() 
come true.

On 10/21/20 10:21 PM, Hui Wang wrote:
>
> 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