[PATCH] Silence Coverity warning in the autofs test tool

[Jakub Hrozek]

I think Coverity is being too noisy here, because only if
_sss_getautomntent_r() succeeds, then key and value are allocated. This
fix should hopefully explain the code to Coverity, too :-)

[Jan Zelený]

> I think Coverity is being too noisy here, because only if
> _sss_getautomntent_r() succeeds, then key and value are allocated. This
> fix should hopefully explain the code to Coverity, too :-)

I have a question regarding the part of code where you delete exit() calls and 
replace them with jumps to end: label. There are two places like this left in 
the code, is that ok? IMO they could also lead to proper cleanup instead of 
just exiting.

Thanks
Jan

[Jakub Hrozek]

On Mon, Mar 26, 2012 at 11:17:37AM +0200, Jan Zelený wrote:
> > I think Coverity is being too noisy here, because only if
> > _sss_getautomntent_r() succeeds, then key and value are allocated. This
> > fix should hopefully explain the code to Coverity, too :-)
>
> I have a question regarding the part of code where you delete exit() calls and 
> replace them with jumps to end: label. There are two places like this left in 
> the code, is that ok? IMO they could also lead to proper cleanup instead of 
> just exiting.
>
> Thanks
> Jan

No, the end: label is a cleanup that triggers endautomntent. The two
exits are called before a successfull setautomntent. We should clean up
the popt context as soon as we are done with processing the options,
though.

New patch is attached.

[Jan Zelený]

> On Mon, Mar 26, 2012 at 11:17:37AM +0200, Jan Zelený wrote:
> > > I think Coverity is being too noisy here, because only if
> > > _sss_getautomntent_r() succeeds, then key and value are allocated. This
> > > fix should hopefully explain the code to Coverity, too :-)
> >
> > I have a question regarding the part of code where you delete exit()
> > calls and replace them with jumps to end: label. There are two places
> > like this left in the code, is that ok? IMO they could also lead to
> > proper cleanup instead of just exiting.
> >
> > Thanks
> > Jan
>
> No, the end: label is a cleanup that triggers endautomntent. The two
> exits are called before a successfull setautomntent. We should clean up
> the popt context as soon as we are done with processing the options,
> though.
>
> New patch is attached.

Yes, that's what I meant, sorry for not being more specific.

Ack

Thanks
Jan

[Stephen Gallagher]

On Tue, 2012-03-27 at 08:13 +0200, Jan Zelený wrote:
> > On Mon, Mar 26, 2012 at 11:17:37AM +0200, Jan Zelený wrote:
> > > > I think Coverity is being too noisy here, because only if
> > > > _sss_getautomntent_r() succeeds, then key and value are allocated. This
> > > > fix should hopefully explain the code to Coverity, too :-)
> > >
> > > I have a question regarding the part of code where you delete exit()
> > > calls and replace them with jumps to end: label. There are two places
> > > like this left in the code, is that ok? IMO they could also lead to
> > > proper cleanup instead of just exiting.
> > >
> > > Thanks
> > > Jan
> >
> > No, the end: label is a cleanup that triggers endautomntent. The two
> > exits are called before a successfull setautomntent. We should clean up
> > the popt context as soon as we are done with processing the options,
> > though.
> >
> > New patch is attached.
>
> Yes, that's what I meant, sorry for not being more specific.
>
> Ack

Pushed to master and sssd-1-8.