[sr-dev] git:tirpi/cfg_framework_multivalue: ctl: rpc-> scan does not immediately send out errors

Andrei Pelinescu-Onciul andrei at iptel.org
Thu Oct 7 11:18:36 CEST 2010


On Oct 05, 2010 at 16:31, Miklos Tirpak <miklos at iptel.org> wrote:
> Hi Andrei,
> 
> if you do not mind, I modified the ctl module a little bit, this
> feature was needed for the cfg.set RPC command to support both int
> and string parameters simultaneously.
> Do you think that it is worth cherry-picking this change along with
> the cfg.set RPC command to the master branch before 3.1 is released?

It should work with xmlrpc too.
Aynway it was too late, but we might backport latter to one of 3.1.x
(since they don't change anything).

Andrei
> 
> On 10/05/2010 04:20 PM, Miklos Tirpak wrote:
> >Module: sip-router
> >Branch: tirpi/cfg_framework_multivalue
> >Commit: 77576163bacf3d25ba5694563df173ce2aa4aa5f
> >URL:    http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=77576163bacf3d25ba5694563df173ce2aa4aa5f
> >
> >Author: Miklos Tirpak<miklos at iptel.org>
> >Committer: Miklos Tirpak<miklos at iptel.org>
> >Date:   Tue Oct  5 16:09:27 2010 +0200
> >
> >ctl: rpc->scan does not immediately send out errors
> >
> >When the rpc scan function cannot read a parameter because of
> >a type mismatch, the function only prepares the error reply but
> >does not send it out immediately. The scan can be retried with
> >different parameter specification which resets the prepared
> >error message.
> >
> >This allows variable types of parameters of the same rpc function,
> >for example:
> >
> >if (rpc->scan(c, "d",&i) == 1)
> >	/* int parameter */
> >else if (rpc->scan(c, "s",&ch) == 1)
> >	/* char* parameter */
> >else
> >	return /* error */
> >
> >---
> >
> >  modules/ctl/binrpc_run.c |  124 ++++++++++++++++++++++++++++++++++++---------
> >  1 files changed, 99 insertions(+), 25 deletions(-)
> >
> >diff --git a/modules/ctl/binrpc_run.c b/modules/ctl/binrpc_run.c
> >index 989f889..4732c19 100644
> >--- a/modules/ctl/binrpc_run.c
> >+++ b/modules/ctl/binrpc_run.c
> >@@ -97,6 +97,8 @@ struct binrpc_ctx{
> >  	char* method;
> >  	struct binrpc_gc_block* gc; /**<  garbage collection */
> >  	int replied;
> >+	int err_code;
> >+	str err_phrase;	/**<  Leading zero must be included! */
> >  };
> >
> >
> >@@ -388,6 +390,10 @@ inline void destroy_binrpc_ctx(struct binrpc_ctx* ctx)
> >  		pkg_free(ctx->out.pkt.body);
> >  		ctx->out.pkt.body=0;
> >  	}
> >+	if (ctx->err_phrase.s){
> >+		pkg_free(ctx->err_phrase.s);
> >+		ctx->err_phrase.s=NULL;
> >+	}
> >  	binrpc_gc_collect(ctx);
> >  }
> >
> >@@ -395,32 +401,23 @@ inline void destroy_binrpc_ctx(struct binrpc_ctx* ctx)
> >
> >  #define MAX_FAULT_LEN 256
> >  #define FAULT_START_BUF (3 /* maxint*/+2/*max str header*/)
> >-static void rpc_fault(struct binrpc_ctx* ctx, int code, char* fmt, ...)
> >+static void _rpc_fault(struct binrpc_ctx* ctx, int code,
> >+			char *phrase, int phrase_len)
> >  {
> >-	char buf[MAX_FAULT_LEN];
> >  	static unsigned char fault_start[FAULT_START_BUF];
> >  	static unsigned char hdr[BINRPC_MAX_HDR_SIZE];
> >  	struct iovec v[3];
> >  	struct binrpc_pkt body;
> >  	int b_len;
> >-	va_list ap;
> >-	int len;
> >  	int hdr_len;
> >  	int err;
> >-	
> >+
> >  	if (ctx->replied){
> >  		LOG(L_ERR, "ERROR: binrpc: rpc_send: rpc method %s tried to reply"
> >  					" more then once\n", ctx->method?ctx->method:"");
> >  		return;
> >  	}
> >  	err=0;
> >-	va_start(ap, fmt);
> >-	len=vsnprintf(buf, MAX_FAULT_LEN, fmt, ap); /* ignore trunc. errors */
> >-	if ((len<0) || (len>  MAX_FAULT_LEN))
> >-		len=MAX_FAULT_LEN-1;
> >-	va_end(ap);
> >-	
> >-	len++; /* vnsprintf doesn't include the terminating 0 */
> >  	err=binrpc_init_pkt(&body, fault_start, FAULT_START_BUF);
> >  	if (err<0){
> >  		LOG(L_ERR, "ERROR: binrpc_init_pkt error\n");
> >@@ -429,22 +426,22 @@ static void rpc_fault(struct binrpc_ctx* ctx, int code, char* fmt, ...)
> >  	/* adding a fault "manually" to avoid extra memcpys */
> >  	err=binrpc_addint(&body, code);
> >  	if (err<0){
> >-		LOG(L_ERR, "ERROR: rpc_fault: addint error\n");
> >+		LOG(L_ERR, "ERROR: _rpc_fault: addint error\n");
> >  		goto error;
> >  	}
> >-	err=binrpc_add_str_mark(&body, BINRPC_T_STR, len);
> >+	err=binrpc_add_str_mark(&body, BINRPC_T_STR, phrase_len);
> >  	if (err<0){
> >-		LOG(L_ERR, "ERROR: rpc_fault: add_str_mark error\n");
> >+		LOG(L_ERR, "ERROR: _rpc_fault: add_str_mark error\n");
> >  		goto error;
> >  	}
> >  	/*
> >-	err=binrpc_addfault(&body, code, buf, len);
> >+	err=binrpc_addfault(&body, code, phrase, phrase_len);
> >  	if (err<0){
> >  		LOG(L_ERR, "ERROR: binrpc_addfault error\n");
> >  		goto error;
> >  	}*/
> >  	b_len=binrpc_pkt_len(&body);
> >-	err=hdr_len=binrpc_build_hdr(BINRPC_FAULT, b_len+len,
> >+	err=hdr_len=binrpc_build_hdr(BINRPC_FAULT, b_len+phrase_len,
> >  								ctx->in.ctx.cookie, hdr, BINRPC_MAX_HDR_SIZE);
> >  	if (err<0){
> >  		LOG(L_ERR, "ERROR: binrpc_build_hdr error\n");
> >@@ -454,25 +451,90 @@ static void rpc_fault(struct binrpc_ctx* ctx, int code, char* fmt, ...)
> >  	v[0].iov_len=hdr_len;
> >  	v[1].iov_base=body.body;
> >  	v[1].iov_len=b_len;
> >-	v[2].iov_base=buf;
> >-	v[2].iov_len=len;
> >+	v[2].iov_base=phrase;
> >+	v[2].iov_len=phrase_len;
> >  	if ((err=sock_send_v(ctx->send_h, v, 3))<0){
> >  		if (err==-2){
> >-			LOG(L_ERR, "ERROR: binrpc_fault: send failed: "
> >+			LOG(L_ERR, "ERROR: _rpc_fault: send failed: "
> >  					"datagram too big\n");
> >  			return;
> >  		}
> >-		LOG(L_ERR, "ERROR: binrpc_fault: send failed\n");
> >+		LOG(L_ERR, "ERROR: _rpc_fault: send failed\n");
> >  		return;
> >  	}
> >  	ctx->replied=1;
> >  	return;
> >  error:
> >-	LOG(L_ERR, "ERROR: binrpc_fault: binrpc_* failed with: %s (%d)\n",
> >+	LOG(L_ERR, "ERROR: _rpc_fault: binrpc_* failed with: %s (%d)\n",
> >  			binrpc_error(err), err);
> >  }
> >
> >+static void rpc_fault(struct binrpc_ctx* ctx, int code, char* fmt, ...)
> >+{
> >+	char buf[MAX_FAULT_LEN];
> >+	va_list ap;
> >+	int len;
> >
> >+	if (ctx->replied){
> >+		LOG(L_ERR, "ERROR: binrpc: rpc_send: rpc method %s tried to reply"
> >+					" more then once\n", ctx->method?ctx->method:"");
> >+		return;
> >+	}
> >+	va_start(ap, fmt);
> >+	len=vsnprintf(buf, MAX_FAULT_LEN, fmt, ap); /* ignore trunc. errors */
> >+	if ((len<0) || (len>  MAX_FAULT_LEN))
> >+		len=MAX_FAULT_LEN-1;
> >+	va_end(ap);
> >+
> >+	len++; /* vnsprintf doesn't include the terminating 0 */
> >+	return _rpc_fault(ctx, code, buf, len);
> >+}
> >+
> >+/* Prepare the error reply without sending out the message */
> >+static int rpc_fault_prepare(struct binrpc_ctx* ctx, int code, char* fmt, ...)
> >+{
> >+	char buf[MAX_FAULT_LEN];
> >+	va_list ap;
> >+	int len;
> >+
> >+	if (ctx->replied){
> >+		LOG(L_ERR, "ERROR: binrpc: rpc_send: rpc method %s tried to reply"
> >+					" more then once\n", ctx->method?ctx->method:"");
> >+		return -1;
> >+	}
> >+	va_start(ap, fmt);
> >+	len=vsnprintf(buf, MAX_FAULT_LEN, fmt, ap); /* ignore trunc. errors */
> >+	if ((len<0) || (len>  MAX_FAULT_LEN))
> >+		len=MAX_FAULT_LEN-1;
> >+	va_end(ap);
> >+
> >+	len++; /* vnsprintf doesn't include the terminating 0 */
> >+
> >+	ctx->err_code = code;
> >+	if (ctx->err_phrase.s)
> >+		pkg_free(ctx->err_phrase.s);
> >+	ctx->err_phrase.s = (char*)pkg_malloc(sizeof(char)*len);
> >+	if (!ctx->err_phrase.s) {
> >+		ctx->err_code = 0;
> >+		ctx->err_phrase.len = 0;
> >+		LOG(L_ERR, "ERROR: rpc_fault_prepare: not enough memory\n");
> >+		return -1;
> >+	}
> >+	memcpy(ctx->err_phrase.s, buf, len);
> >+	ctx->err_phrase.len = len;
> >+	return 0;
> >+}
> >+
> >+/* Reset the saved error code */
> >+static void rpc_fault_reset(struct binrpc_ctx* ctx)
> >+{
> >+	ctx->err_code = 0;
> >+	if (ctx->err_phrase.s) {
> >+		pkg_free(ctx->err_phrase.s);
> >+		ctx->err_phrase.s = NULL;
> >+		ctx->err_phrase.len = 0;
> >+	}
> >+}
> >
> >  /* build the reply from the current body */
> >  static int rpc_send(struct binrpc_ctx* ctx)
> >@@ -596,14 +658,21 @@ int process_rpc_req(unsigned char* buf, int size, int* bytes_needed,
> >  	f_ctx.method=val.u.strval.s;
> >  	rpc_e->function(&binrpc_callbacks,&f_ctx);
> >  	if (f_ctx.replied==0){
> >+		if ((binrpc_pkt_len(&f_ctx.out.pkt)==0)
> >+			&&  f_ctx.err_code&&  f_ctx.err_phrase.s
> >+		) {
> >+			_rpc_fault(&f_ctx, f_ctx.err_code,
> >+				f_ctx.err_phrase.s, f_ctx.err_phrase.len);
> >  		/* to get an error reply if the rpc handlers hasn't replied
> >  		 *  uncomment the following code:
> >-		 * if (binrpc_pkt_len(&f_ctx.out.pkt)==0){
> >+		 * } else if (binrpc_pkt_len(&f_ctx.out.pkt)==0){
> >  			rpc_fault(&f_ctx, 500, "internal server error: no reply");
> >  			LOG(L_ERR, "ERROR: rpc method %s hasn't replied\n",
> >  					val.u.strval.s);
> >-		}else  */
> >+		 */
> >+		} else {
> >  			rpc_send(&f_ctx);
> >+		}
> >  	}
> >  end:
> >  	*bytes_needed=0; /* full read */
> >@@ -749,6 +818,9 @@ static int rpc_scan(struct binrpc_ctx* ctx, char* fmt, ...)
> >  	double d;
> >  	str* s;
> >  	
> >+	/* clear the previously saved error code */
> >+	rpc_fault_reset(ctx);
> >+
> >  	va_start(ap, fmt);
> >  	orig_fmt=fmt;
> >  	nofault = 0;
> >@@ -823,8 +895,10 @@ static int rpc_scan(struct binrpc_ctx* ctx, char* fmt, ...)
> >  	va_end(ap);
> >  	return (int)(fmt-orig_fmt)-modifiers;
> >  error_read:
> >+	/* Do not immediately send out the error message, the user might retry the scan with
> >+	different parameters */
> >  	if(nofault==0 || ((err!=E_BINRPC_MORE_DATA)&&  (err!=E_BINRPC_EOP)))
> >-		rpc_fault(ctx, 400, "error at parameter %d: expected %s type but"
> >+		rpc_fault_prepare(ctx, 400, "error at parameter %d: expected %s type but"
> >  						" %s", ctx->in.record_no, rpc_type_name(v.type),
> >  						 binrpc_error(err));
> >  	/*
> >
> >
> >_______________________________________________
> >sr-dev mailing list
> >sr-dev at lists.sip-router.org
> >http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev



More information about the sr-dev mailing list