[PATCH] RFC debug detect winbindd version mismatches

[Andrew Bartlett]

On Mon, 2014-01-27 at 15:36 +0100, Volker Lendecke wrote:
> On Mon, Jan 27, 2014 at 02:25:09PM +0000, Noel Power wrote:
> > > I'm not opposed to this feature. Question: Is there a way to
> > > do this without a global variable?
> > everything is possible :-)
> > >  Maybe with a special call
> > > that queries the currently available interface version
> > unfortunately 'winbind_open_pipe_sock' has a version request as part of
> > the connection setup, if the version check fails then then you have no
> > chance to get the result of a subsequent version request (the way the
> > code currently is written)
> 
> Well, then rewrite the code...
> 
> You might factor out the version retrieval and call that
> from a different path. I know that wb_common is a big mess
> which needs cleaning up. I've tried to make that properly
> async at some point but gave up.
> 
> > yup, understood, wasn't completely in love with the solution myself but
> > seemed the easiest less impactful way to achieve this (mostly debug
> > relevant) change, I can rewrite this by changing
> > winbind_open_pipe_sock/winbind_read_sock/winbind_write_sock (and
> > associated call chain) if the consensus is that it is necessary
> 
> Not sure about consensus, I'm just uttering my personal
> opinion here. I could not even block the global variable,
> but I would just like to not see that go in this way.

I tend to agree here.  The concept is quite useful, but we need to
improve the implementation.  I don't see why this can't be passed back
as an extra pointer argument for example, of the server version if
obtained.  That layer isn't in an ABI, it can be reworked, and
static/global variables almost always bite us in the end. 

Either way, declaring the prototype in another C file strictly against
our coding standards:
+int winbindd_get_mismatched_version(void);

But the other issue I see is that this will only work if the structure
size hasn't changed.  If that changes, we are back to square one - or
worse, a timeout waiting for the last few bytes in the request/reply.
To really do this properly, we need either a better encoding or at least
a different message type for the version check that doesn't involve the
struct sizes directly.

Sorry,

Andrew Bartlett

-- 
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba