[PATCH] Add messaging_init_client() function

Jeremy Allison jra at samba.org
Mon Nov 14 18:33:08 UTC 2016


On Mon, Nov 14, 2016 at 12:16:43PM +0100, Andreas Schneider wrote:
> On Monday, 14 November 2016 11:51:59 CET Volker Lendecke wrote:
> > On Mon, Nov 14, 2016 at 11:42:01AM +0100, Andreas Schneider wrote:
> > > The attached patchset adds a messaging_init_client() to correctly address
> > > issues when we try to setup messaging in a client tool like rpcclient or
> > > net.
> > > 
> > > It also cleans them up a bit.
> > > 
> > > Review and push much appreciated!
> > 
> > In general I like those changes, just returning a pointer is
> > definitely not enough.
> > 
> > However -- is returning NT_STATUS_OK really the right thing for the
> > client_init function when it did not work? Shouldn't we lift the error
> > handling a bit up? NT_STATUS_OK tells the caller everything was fine,
> > and I would expect crashes later on trying to dereference the
> > messaging context pointer.
> 
> Check the code, the messaging context can be NULL in the client code!
> 
> I recently did a change which broke running rpcclient running as user in 
> master.
> 
> https://git.samba.org/?
> p=samba.git;a=commitdiff;h=fb155a946d995de2c518d22bdb44f3d7f0b8dca3
> 
> This change fix it so, if we cannot access the locking direction we do not 
> fail and continue to work. If we can access the directory, so we are root we 
> will fail if something goes wrong beyond the directory access!

Yeah, but Volker is right in that returning NT_STATUS_OK isn't correct
when it didn't work.

Let the client code calling handle NT_STATUS_ACCESS_DENIED and a
NULL messaging context, don't pretent it worked.

I'll take a look at this - sorry about missing rpcclient failure
in fb155a946d995de2c518d22bdb44f3d7f0b8dca3.

Jeremy.



More information about the samba-technical mailing list