[PATCH] Fix / slightly simplify smbd/scavenger

[Jeremy Allison]

On Fri, Oct 30, 2015 at 05:09:03PM +0100, Volker Lendecke wrote:
> On Fri, Oct 30, 2015 at 09:07:07AM -0700, Jeremy Allison wrote:
> > On Fri, Oct 30, 2015 at 04:57:39PM +0100, Volker Lendecke wrote:
> > > On Fri, Oct 30, 2015 at 08:34:45AM -0700, Jeremy Allison wrote:
> > > > On Fri, Oct 30, 2015 at 07:46:39AM +0100, Ralph Boehme wrote:
> > > > > On Thu, Oct 29, 2015 at 03:26:53PM -0700, Jeremy Allison wrote:
> > > > > > On Thu, Oct 29, 2015 at 08:23:26PM +0200, Uri Simchoni wrote:
> > > > > > > 
> > > > > > > 
> > > > > > > On 10/29/2015 06:02 PM, Volker Lendecke wrote:
> > > > > > > >+	ret = read_data(fd, child, sizeof(child));
> > > > > > > >+	if (ret == -1) {
> > > > > > > Doesn't seem to take EOF into account, but neither does the original
> > > > > > > code. How come?
> > > > > > 
> > > > > > fd's are created by socketpair() and not set non-blocking,
> > > > > > so shouldn't get a zero return.
> > > > > 
> > > > > But wouldn't one side get 0 when the other does a close?
> > > > 
> > > > Yeah, but I thought these were long-lived sockets (only
> > > > closed on shutdown). I'll look again.
> > > 
> > > Ok, this goes too far. Attached find a minimal patch that
> > > just fixes the very obvious bug.
> > > 
> > > Maybe that's a bit less controversial.
> > 
> > It's not controversy, it's people trying to
> > remember how code works :-).
> > 
> > > Review&push appreciated!
> > 
> > Nah, first patch is better. Pushed that one :-).
> 
> Which one? There's 3 versions floating around now. One with
> just the read_data, one with the read_data an short-read
> protection and then now the minimal one.

The minimal one doesn't tidy up the code like the second
one does, that's why I prefer the second one.

However, the second one has a bug :-). In scavenger_wait_hello()

static bool scavenger_wait_hello(int fd, struct server_id *child)

-       size_t remaining = sizeof(*child);

which reads sizeof(struct server_id).

and in the new code :

+       ret = read_data(fd, child, sizeof(child));

Which reads sizeof(struct server_id *).

Fixed version follows. If no objections I'll push
shortly.

Jeremy.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-smbd-Fix-simplify-scavenger-routines.patch
Type: text/x-diff
Size: 2566 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20151030/6abb7033/0001-smbd-Fix-simplify-scavenger-routines.diff>