[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