[PATCH] Address some -O3 issues on Ubuntu 10.04

[Michael Adam]

On 2016-08-04 at 12:35 -0700, Jeremy Allison wrote:
> On Thu, Aug 04, 2016 at 09:29:13PM +0200, Michael Adam wrote:
> > On 2016-08-04 at 08:56 -0700, Jeremy Allison wrote:
> > > On Thu, Aug 04, 2016 at 11:38:33AM +0200, Michael Adam wrote:
> > > > On 2016-08-04 at 00:39 +0200, Michael Adam wrote:
> > > > > On 2016-08-03 at 17:41 +0200, Stefan Metzmacher wrote:
> > > > > > Hi,
> > > > > > 
> > > > > > here's an update that also includes andrew's backupkey test fixes.
> > > > > > 
> > > > > > All with bug references for the backports.
> > > > > > 
> > > > > > Please review and push.
> > > > > > 
> > > > > > (I already pushed the two patches from Andrew)
> > > > > 
> > > > > Reviewed-by: me and pushed to autobuild.
> > > > 
> > > > Re-pushed after failed autobuild.
> > > 
> > > Yes, mine and Andreas's autobuild failed also.
> > > I'll re-push one more time. Ralph seemed to
> > > get something through (god knows how :-).
> > 
> > Yeah... some time ago. ;-)
> > 
> > Metze's last autobuild failed as well.
> > 
> > Mine is currently running (at this moment at the drs tests...)
> > and it has metze's last autobuild content included.
> > Andreas' autobuild is in the queue mirroring mine.
> > 
> > > I think if metze recent work doesn't fix it the
> > > samba4.*samba_tool* tests need to be skipped
> > > until fixed.
> > 
> > This!
> 
> Well after metze's analysis I have one final attempt
> at fixing them (attached). I think patch #1 is a
> genuine bugfix that needs to go in regardless.

Good! But see 1 comment inline below.

> Patch #2 is a band-aid for the error, but something
> like it is needed to stop recursion (which currently
> isn't prevented in the repl code at all).

I somehow kind of like that one. :-)

I'd be happy to review+ it in if it fixes our problems
for now, but if other, more drs/ad-prone engineers could
give their 2 cents, I'd be more comfortable. :-)

> If you want to review and try pushing them I won't
> object :-).

First these comments, and now waiting for my autobuild
before pushing again... (at 1937(14754)/1951 at 3h11m40s ...)
fingers crossed...

Cheers - Michael

See comment inline below:

> From a58d4eb3cd62156291468d648d23ff473d0202e8 Mon Sep 17 00:00:00 2001
> From: Jeremy Allison <jra at samba.org>
> Date: Thu, 4 Aug 2016 11:09:21 -0700
> Subject: [PATCH 1/2] s4: repl: Ensure all error paths in
>  dreplsrv_op_pull_source_get_changes_trigger() are protected with tevent
>  returns.
> 
> Otherwise dreplsrv_op_pull_source_get_changes_trigger() could infinitely recurse.
> 
> Signed-off-by: Jeremy Allison <jra at samba.org>
> ---
>  source4/dsdb/repl/drepl_out_helpers.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/source4/dsdb/repl/drepl_out_helpers.c b/source4/dsdb/repl/drepl_out_helpers.c
> index bf8a372..29bb9d5 100644
> --- a/source4/dsdb/repl/drepl_out_helpers.c
> +++ b/source4/dsdb/repl/drepl_out_helpers.c
> @@ -446,6 +446,9 @@ static void dreplsrv_op_pull_source_get_changes_trigger(struct tevent_req *req)
>  		if (!W_ERROR_IS_OK(werr)) {
>  			DEBUG(0,(__location__ ": Failed to convert UDV for %s : %s\n",
>  				 ldb_dn_get_linearized(partition->dn), win_errstr(werr)));
> +			/* The above error can only be OUT_OF_MEMORY. */
> +			tevent_req_nterror(req, NT_STATUS_NO_MEMORY);
> +			return;

Why not use tevent_req_werror(req, werr) ?

>  		}
>  	}
>  
> @@ -470,6 +473,7 @@ static void dreplsrv_op_pull_source_get_changes_trigger(struct tevent_req *req)
>  		status = dreplsrv_get_gc_partial_attribute_set(service, r, &pas);
>  		if (!NT_STATUS_IS_OK(status)) {
>  			DEBUG(0,(__location__ ": Failed to construct GC partial attribute set : %s\n", nt_errstr(status)));
> +			tevent_req_nterror(req, status);
>  			return;
>  		}
>  		replica_flags &= ~DRSUAPI_DRS_WRIT_REP;
> @@ -482,6 +486,7 @@ static void dreplsrv_op_pull_source_get_changes_trigger(struct tevent_req *req)
>  		status = dreplsrv_get_rodc_partial_attribute_set(service, r, &pas, for_schema);
>  		if (!NT_STATUS_IS_OK(status)) {
>  			DEBUG(0,(__location__ ": Failed to construct RODC partial attribute set : %s\n", nt_errstr(status)));
> +			tevent_req_nterror(req, status);
>  			return;
>  		}
>  		replica_flags &= ~DRSUAPI_DRS_WRIT_REP;
> -- 
> 2.8.0.rc3.226.g39d4020

Apart from the question above, reviewed-by: me

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20160804/b7045b74/signature.sig>