[PATCH] Do not dereference a NULL pointer in tsocket

Jeremy Allison jra at samba.org
Thu Jun 23 17:06:44 UTC 2016


On Thu, Jun 23, 2016 at 08:19:17AM +0200, Andreas Schneider wrote:
> On Wednesday, 22 June 2016 20:14:36 CEST Stefan Metzmacher wrote:
> > Am 22.06.2016 um 19:15 schrieb Jeremy Allison:
> > > On Wed, Jun 22, 2016 at 03:39:28PM +0200, Andreas Schneider wrote:
> > >> Review and push appreciated!
> > > 
> > > Should this be
> > > 
> > > if (lrbsda != NULL) {
> > > 
> > > instead of:
> > > 
> > > if (is_inet) {
> > 
> > This is not needed at all!
> > 
> > The code looks like this:
> > 
> >         if (!state->local) {
> >                 tevent_req_done(req);
> >                 goto post;
> >         }
> > 
> >         ret = getsockname(state->fd, &lrbsda->u.sa, &lrbsda->sa_socklen);
> >         if (ret == -1) {
> >                 tevent_req_error(req, errno);
> >                 goto post;
> >         }
> > 
> > And state->local is NULL unless is_inet is set.
> > 
> > I think we should not blindly fix coverity bugs,
> > when there's no real problem.

Personally I think coverity bugs, even when they're
false positives, point at areas where the code is
unclear and where mistakes may arise in future
modifications.

Look at the above:

"And state->local is NULL unless is_inet is set."

That's a current constraint - but later modifications
may break it. Changing things like this to make
the intent match the code is a good cleanup to
do IMHO.

> > We may change if (!state->local) into if (lrbsda == NULL),
> > maybe that makes coverity more happy.
> 
> I'm fine with that but I'm also ok not patching it.
> 
> -- 
> Andreas Schneider                   GPG-ID: CC014E3D
> Samba Team                             asn at samba.org
> www.samba.org
> 



More information about the samba-technical mailing list