[PATCH] Address some -O3 issues on Ubuntu 10.04
Jeremy Allison
jra at samba.org
Thu Aug 4 20:49:32 UTC 2016
On Thu, Aug 04, 2016 at 09:57:22PM +0200, Michael Adam wrote:
> > 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. :-)
Yeah, that would be welcome. Something like this is needed,
even if not exactly this code.
Right now there's nothing to stop the loop I mentioned
in the previous comment from causing the server to spin.
> 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) ?
Because I'd forgotten it existed :-). Good catch - thanks !
I'll push this (with the fixup).
> > @@ -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
More information about the samba-technical
mailing list