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

Miklos Tirpak miklos at iptel.org
Thu Oct 7 12:42:34 CEST 2010


Hi Andrei,

On 10/07/2010 11:18 AM, Andrei Pelinescu-Onciul wrote:
> 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.

I just tried it, and it partially works: cfg.set can set the values 
properly, but the xmlrpc response is an error sometimes depending on the 
parameter type. I will fix this later.

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

OK, no problem.

Thanks,
Miklos

>
> 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