[PATCH] sink-input: Don't assert when removing non-existent volume factor

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 2013-08-05 at 09:24 +0300, Tanu Kaskinen wrote:
> On Mon, 2013-08-05 at 11:44 +0530, Arun Raghavan wrote:
> > On Mon, 2013-08-05 at 09:03 +0300, Tanu Kaskinen wrote:
> > > On Sat, 2013-08-03 at 11:41 +0530, Arun Raghavan wrote:
> > > > This makes it easier for users of this API to add/updated a volume
> > > > factor by doing a _remove_volume_factor() followed by an
> > > > add_volume_factor(), rather than having to either remember whether this
> > > > is the first set operation or have an API to query whether a factor has
> > > > already been set.
> > > 
> > > There is a query API already:
> > > pa_hashmap_contains(i->volume_factor_items, key)
> > > 
> > > Oops, I was wrong, that function doesn't exist (it probably should be
> > > added). The same thing can be achieved with pa_hashmap_get() too,
> > > though.
> > 
> > I think it's a bit ugly to expect users of that API to check the
> > underlying structure itself first, though.
> 
> Yes, but only slightly ugly. We generally allow reading object variables
> directly across classes (and to some extent write, but in my opinion
> that shouldn't be allowed).
> 
> Anyway, I changed my mind about the change even before your reply. The
> reason is that I think it's ugly to require callers to always check the
> object state before calling a function. The function should fail if the

Yup, that was my primary reason.

> state is not good (fail in some other way than crashing, that is). I'd
> like the failure to be reflected in the return value, so could you
> change the void function to return an int?

Sure, it's en route. Used a bool, though.

-- Arun



[Index of Archives]     [Linux Audio Users]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux