Re: [PATCH] coreaudio: Always return 0 in handle_voice_change

[Akihiko Odaki]

On Sun, Mar 6, 2022 at 9:16 PM Christian Schoenebeck
<qemu_oss@crudebyte.com> wrote:
>
> On Sonntag, 6. März 2022 11:54:00 CET Akihiko Odaki wrote:
> > On 2022/03/06 19:49, Christian Schoenebeck wrote:
> > > On Sonntag, 6. März 2022 07:39:49 CET Akihiko Odaki wrote:
> > >> MacOSX.sdk/System/Library/Frameworks/CoreAudio.framework/Headers/AudioHar
> > >> dwa re.h
> > >>
> > >> says:
> > >>> The return value is currently unused and should always be 0.
> > >
> > > Where does it say that, about which macOS functions? There are quite a
> > > bunch of macOS functions involved in init_out_device(), and they all have
> > > error pathes in init_out_device(), and they still will have them after
> > > this patch.
> > >
> > > And again, I'm missing: this as an improvement because? Is this a user
> > > invisible improvement (e.g. removing redundant branches), or is this a
> > > user
> > > visible improvement, i.e. does it fix a misbehaviour? In case of the
> > > latter, which misbehaviour did you encounter?
> >
> > handle_voice_change itself is a callback.
> > It is invisible for a user since "the return value is currently unused".
>
> Then the commit log should be more specific and say something like:
>
> "
> handle_voice_change() is a CoreAudio callback function as of CoreAudio type
> 'AudioObjectPropertyListenerProc', and for the latter MacOSX.sdk/System/
> Library/Frameworks/CoreAudio.framework/Headers/AudioHardware.h
> says 'The return value is currently unused and should always be 0.'.

That makes sense. I'll submit v2 soon.

> "
>
> Nevertheless, personally I would not change that, but I won't object either.
>
> I read it like "The CoreAudio subsystem of macOS currently ignores the result
> of your callback, and for that reason simply return 0 for now.". It does not
> say "you must not return anything else than 0". ATM it simply does not matter
> what you return here.

I think it is dangerous to guess something not described in the
comment. It can lead to possible breakage if the guess turns out false
and a future Core Audio version decides to do something unintended
when it is not 0. OStatus is just an integer type and does not have a
particular semantics like enum types and errno so it is impossible to
know possible consequences in the case. Returning 0 as documented is
possibly safer and not harmful at least.

Regards,
Akihiko Odaki

>
> Best regards,
> Christian Schoenebeck
>
>