[pulseaudio-discuss] [PATCH 1/4] sink, source: Assign to reference_volume from only one place

[Tanu Kaskinen]

On Mon, 2014-04-07 at 19:59 +0600, Alexander E. Patrakov wrote:
> 07.04.2014 18:24, Tanu Kaskinen wrote:
> >   
> > +/* Called from the main thread. */
> > +static void set_reference_volume_internal(pa_sink *s, const pa_cvolume *volume) {
> > +    pa_cvolume old_volume;
> > +    char old_volume_str[PA_CVOLUME_SNPRINT_VERBOSE_MAX];
> > +    char new_volume_str[PA_CVOLUME_SNPRINT_VERBOSE_MAX];
> > +
> > +    pa_assert(s);
> > +    pa_assert(volume);
> 
> Note the assertions.
> 
> > +
> > +/* Called from the main thread. */
> > +void pa_sink_set_reference_volume_direct(pa_sink *s, const pa_cvolume *volume) {
> > +    pa_assert(s);
> > +    pa_assert(volume);
> > +
> > +    set_reference_volume_internal(s, volume);
> > +}
> 
> What's the point of having this function in this patch? As demonstrated 
> above, set_reference_volume_internal does the same assertions.

The "point" is in the function naming: I like to have a function called
set_reference_volume_internal(), but it makes no sense to have a
function with that name exported outside the .c file, so I added another
function for interfacing with the outside world. I can't give good
reasons for why I like the name set_reference_volume_internal(), so I
think it's best that I drop set_reference_volume_internal() and move the
implementation inside pa_sink_set_reference_volume_direct().

> Same for sources.
> 
> The patch does look like an equivalent refactoring.

Thanks for the review!

-- 
Tanu