[pulseaudio-discuss] [PATCH] device-manager: Add an assertion to get rid of a warning from Coverity.

Arun Raghavan arun.raghavan at collabora.co.uk
Wed Mar 28 09:13:33 PDT 2012


On Wed, 2012-03-28 at 14:14 +0300, Tanu Kaskinen wrote:
> On Wed, 2012-03-28 at 16:30 +0530, Arun Raghavan wrote:
> > On Wed, 2012-03-28 at 12:27 +0300, Tanu Kaskinen wrote:
> > > Coverity thinks that device_name can be NULL when it's
> > > dereferenced by strcmp. Adding an assertion doesn't hurt
> > > here (in my opinion), and that should get rid of the
> > > warning.
> > > ---
> > >  src/modules/module-device-manager.c |    2 +-
> > >  1 files changed, 1 insertions(+), 1 deletions(-)
> > > 
> > > diff --git a/src/modules/module-device-manager.c b/src/modules/module-device-manager.c
> > > index 2ce4c78..e11921d 100644
> > > --- a/src/modules/module-device-manager.c
> > > +++ b/src/modules/module-device-manager.c
> > > @@ -576,7 +576,7 @@ static void update_highest_priority_device_indexes(struct userdata *u, const cha
> > >              struct entry *e;
> > >  
> > >              name = pa_xstrndup(key.data, key.size);
> > > -            device_name = get_name(name, prefix);
> > > +            pa_assert_se(device_name = get_name(name, prefix));
> > >  
> > >              if ((e = entry_read(u, name))) {
> > >                  for (uint32_t i = 0; i < NUM_ROLES; ++i) {
> > 
> > This looks like a legitimate warning (f.ex. if the db has bad data). So
> > the condition should look like:
> > 
> >     if (device_name && (e = entry_read(u, name))
> 
> get_name() fails if name doesn't start with prefix. This code is inside
> an if block that has the following condition:
> 
>         if (key.size > strlen(prefix) && strncmp(key.data, prefix, strlen(prefix)) == 0)
> 
> So we know that key starts with prefix. name is a copy of key, so we
> know that name starts with prefix. Therefore get_name() can't fail, and
> the warning was a false positive.

Ah, indeed. I see you've pushed other Coverity fixes as well -- cool!
Are these the worst offenders? If yes, that's quite neat. :)

-- Arun



More information about the pulseaudio-discuss mailing list