[PATCHES] some new tevent features

[Volker Lendecke]

On Mon, Jan 13, 2014 at 09:11:42PM +0100, Stefan (metze) Metzmacher wrote:
> Hi Volker,
> 
> >>>> static int smbXcli_req_destructor(struct tevent_req *req)
> >>>> static int fncall_destructor(struct tevent_req *req)
> >>>> static int tldap_msg_destructor(struct tevent_req *req)
> >>>
> >>> Thanks for checking. Maybe I was a bit worried because 2 of
> >>> those are mine ... :-)
> >>
> >> Can you review the attached patches?
> > 
> > Please don't push this patches yet, I'm currently debugging a possible
> > problem...
> 
> There was a bug in the interaction with tevent_req_defer_callback()
> and tevent_req_set_cleanup_fn().
> 
> This bug is fixed in the attached patchset (it's also a bit cleaner this
> way).
> We rely the tevent_req_state enum being sorted, we only trigger
> the cleanup function once per state and remember the last state
> and skip the cleanup function on the 2nd call.

Pushed to master with a little re-wording in doxygen:

--- a/lib/tevent/tevent.h
+++ b/lib/tevent/tevent.h
@@ -949,7 +949,7 @@ typedef void (*tevent_req_cleanup_fn)(struct tevent_req *req,
  * @brief This function sets a cleanup function for the given tevent request.
  *
  * This function can be used to setup a cleanup function for the given request.
- * This will be triggered if the tevent_req_done() or tevent_req_error()
+ * This will be triggered when the tevent_req_done() or tevent_req_error()
  * function was called, before notifying the callers callback function,
  * and also before scheduling the deferred trigger.
  *
@@ -957,10 +957,10 @@ typedef void (*tevent_req_cleanup_fn)(struct tevent_req *req,
  * and need to finish both requests at the same time.
  *
  * The cleanup function is able to call tevent_req_done() or tevent_req_error()
- * recursive, the cleanup function is only triggered the first time.
+ * recursively, the cleanup function is only triggered the first time.
  *
  * The cleanup function is also called by tevent_req_received()
- * (may triggered from tevent_req_destructor()) before destroying
+ * (possibly triggered from tevent_req_destructor()) before destroying
  * the private data of the tevent_req.
  *
  * @param[in]  req      The request to use.

Regarding the code, I'd like to see the attached patch also
in. I always try to avoid deep nesting, and directly nested
if-clauses an be replaced with &&.

Please review & push!

Thanks,

Volker

-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:kontakt at sernet.de
-------------- next part --------------
From 8b64b0725fc010d477758094e8e81e8ba3807f2c Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 17 Jan 2014 10:47:29 +0000
Subject: [PATCH] tevent: Avoid one nesting level in cleanup handling

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 lib/tevent/tevent_req.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/lib/tevent/tevent_req.c b/lib/tevent/tevent_req.c
index c48fa58..46719f8 100644
--- a/lib/tevent/tevent_req.c
+++ b/lib/tevent/tevent_req.c
@@ -126,11 +126,10 @@ static void tevent_req_finish(struct tevent_req *req,
 	req->internal.state = state;
 	req->internal.finish_location = location;
 
-	if (req->private_cleanup.fn != NULL) {
-		if (req->private_cleanup.state < req->internal.state) {
-			req->private_cleanup.state = req->internal.state;
-			req->private_cleanup.fn(req, req->internal.state);
-		}
+	if ((req->private_cleanup.fn != NULL) &&
+	    (req->private_cleanup.state < req->internal.state)) {
+		req->private_cleanup.state = req->internal.state;
+		req->private_cleanup.fn(req, req->internal.state);
 	}
 
 	_tevent_req_notify_callback(req, location);
@@ -229,11 +228,10 @@ void tevent_req_received(struct tevent_req *req)
 
 	req->internal.state = TEVENT_REQ_RECEIVED;
 
-	if (req->private_cleanup.fn != NULL) {
-		if (req->private_cleanup.state < req->internal.state) {
-			req->private_cleanup.state = req->internal.state;
-			req->private_cleanup.fn(req, req->internal.state);
-		}
+	if ((req->private_cleanup.fn != NULL) &&
+	    (req->private_cleanup.state < req->internal.state)) {
+		req->private_cleanup.state = req->internal.state;
+		req->private_cleanup.fn(req, req->internal.state);
 	}
 
 	TALLOC_FREE(req->data);
-- 
1.8.1.2