You are viewing a plain text version of this content. The canonical link for it is here.
Posted to c-dev@axis.apache.org by Dimuthu Gamage <di...@gmail.com> on 2008/06/10 04:57:47 UTC
Question about AXIS2_PARAM_CHECK
Hi devs,
Currently in AXIS2_PARAM_CHECK, if the param is null we set the status code
to AXIS2_FAILURE and if not null set the status code AXIS2_SUCCESSS
#define AXIS2_PARAM_CHECK(error, object, error_return) \
if (!object) \
{ \
AXIS2_ERROR_SET_ERROR_NUMBER(error, AXIS2_ERROR_INVALID_NULL_PARAM);
\
AXIS2_ERROR_SET_STATUS_CODE(error, AXIS2_FAILURE); \
return error_return; \
} \
else \
{ \
AXIS2_ERROR_SET_STATUS_CODE(error, AXIS2_SUCCESS); \
}
My question is, is it ok to set the status code to AXIS2_SUCCESS,
Because
1. If we are in happy path, the status code is already AXIS2_SUCCESS, we
don't need to explicitly set that.
2. The macro can overwrite the status, That is if the status is already set
to failure by an early case, this macro will overwrite the status to SUCCESS
which is wrong.
So my suggestion is to remove the else part from that macro?. Please let me
know your ideas, I may be wrong on this.
Thanks
Dimuthu
Re: Question about AXIS2_PARAM_CHECK
Posted by Supun Kamburugamuva <su...@gmail.com>.
Hi,
+1 for removing the else part. When I did profiling the hit count of this
AXIS2_ERROR_SET_STATUS_CODE is very high and it takes a considerable amount
of CPU time as well.
Supun..
On Mon, Jun 9, 2008 at 7:57 PM, Dimuthu Gamage <di...@gmail.com> wrote:
> Hi devs,
>
> Currently in AXIS2_PARAM_CHECK, if the param is null we set the status code
> to AXIS2_FAILURE and if not null set the status code AXIS2_SUCCESSS
>
> #define AXIS2_PARAM_CHECK(error, object, error_return) \
> if (!object) \
> { \
> AXIS2_ERROR_SET_ERROR_NUMBER(error,
> AXIS2_ERROR_INVALID_NULL_PARAM); \
> AXIS2_ERROR_SET_STATUS_CODE(error, AXIS2_FAILURE); \
> return error_return; \
> } \
> else \
> { \
> AXIS2_ERROR_SET_STATUS_CODE(error, AXIS2_SUCCESS); \
> }
>
>
> My question is, is it ok to set the status code to AXIS2_SUCCESS,
> Because
> 1. If we are in happy path, the status code is already AXIS2_SUCCESS, we
> don't need to explicitly set that.
> 2. The macro can overwrite the status, That is if the status is already set
> to failure by an early case, this macro will overwrite the status to SUCCESS
> which is wrong.
>
> So my suggestion is to remove the else part from that macro?. Please let me
> know your ideas, I may be wrong on this.
>
> Thanks
> Dimuthu
>
Re: Question about AXIS2_PARAM_CHECK
Posted by Dimuthu Gamage <di...@gmail.com>.
Hi Damitha,
After reading you explanation, I thought the best way to go is keeping the
status before calling to z() function (in your example) and set the status
back to the early status after calling z. But according to Spuns explanation
this will more degrade the performance.
Anyway for the time being I will go with the former approach for my work.
Thanks
Dimuthu
On Wed, Jun 11, 2008 at 11:47 AM, Supun Kamburugamuva <su...@gmail.com>
wrote:
> Hi Damitha,
>
> Do we have lots of code written in the way that you have mentioned? If yes,
> we may have to keep the existing code. But as you have mentioned previously
> if we are neglecting a error and calling another function that implies two
> things. Actually error is not an error that we should be concerned of or we
> are doing bad coding. In both cases we should remove the unnecessary else
> part.
>
> I have done a profiling recently and it shows that the function with the
> greatest hit count is axutil_error_set_status_code. This function takes so
> much time that it is around half the time of what guththila_next has taken
> (guththila_next is a function with a very high workload).
>
> Regards,
> Supun..
>
>
> On Tue, Jun 10, 2008 at 1:34 AM, Damitha Kumarage <da...@gmail.com>
> wrote:
>
>> Hi,
>> I had the same concern and tried a similar kind of fix few weeks back. But
>> because of the following explanation it seems we have to bear with the
>> current code.
>>
>> Suppose we have implemented your fix. say we call to functions x, y and z
>> in that order. Say inside function x it set an error code status.
>>
>> Now we call y. Inside y() we neglect the error code and say we do even a
>> database commit.
>> Now we call to z(). At the beginning of z say we do an param check. If all
>> parameters are good to go it should pass param checks. But it return an
>> error (because of previously set error status in function x) even when all
>> parameters are good. This is unfair for function z() and it is also
>> misleading.
>>
>> So the best thing to do is to handle error status before calling any
>> important function which has param checkings etc. For example in function
>> y() we have even done a database commit neglecting the error status which
>> is not good.
>>
>> thanks
>> Damitha.
>>
>>
>> Dimuthu Gamage wrote:
>>
>> Hi devs,
>>>
>>> Currently in AXIS2_PARAM_CHECK, if the param is null we set the status
>>> code to AXIS2_FAILURE and if not null set the status code AXIS2_SUCCESSS
>>>
>>> #define AXIS2_PARAM_CHECK(error, object, error_return) \
>>> if (!object) \
>>> { \
>>> AXIS2_ERROR_SET_ERROR_NUMBER(error,
>>> AXIS2_ERROR_INVALID_NULL_PARAM); \
>>> AXIS2_ERROR_SET_STATUS_CODE(error, AXIS2_FAILURE); \
>>> return error_return; \
>>> } \
>>> else \
>>> { \
>>> AXIS2_ERROR_SET_STATUS_CODE(error, AXIS2_SUCCESS); \
>>> }
>>>
>>>
>>> My question is, is it ok to set the status code to AXIS2_SUCCESS,
>>> Because
>>> 1. If we are in happy path, the status code is already AXIS2_SUCCESS, we
>>> don't need to explicitly set that.
>>> 2. The macro can overwrite the status, That is if the status is already
>>> set to failure by an early case, this macro will overwrite the status to
>>> SUCCESS which is wrong.
>>>
>>> So my suggestion is to remove the else part from that macro?. Please let
>>> me know your ideas, I may be wrong on this.
>>>
>>> Thanks
>>> Dimuthu
>>>
>>
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: axis-c-dev-unsubscribe@ws.apache.org
>> For additional commands, e-mail: axis-c-dev-help@ws.apache.org
>>
>>
>
Re: Question about AXIS2_PARAM_CHECK
Posted by Supun Kamburugamuva <su...@gmail.com>.
Hi Damitha,
Do we have lots of code written in the way that you have mentioned? If yes,
we may have to keep the existing code. But as you have mentioned previously
if we are neglecting a error and calling another function that implies two
things. Actually error is not an error that we should be concerned of or we
are doing bad coding. In both cases we should remove the unnecessary else
part.
I have done a profiling recently and it shows that the function with the
greatest hit count is axutil_error_set_status_code. This function takes so
much time that it is around half the time of what guththila_next has taken
(guththila_next is a function with a very high workload).
Regards,
Supun..
On Tue, Jun 10, 2008 at 1:34 AM, Damitha Kumarage <da...@gmail.com>
wrote:
> Hi,
> I had the same concern and tried a similar kind of fix few weeks back. But
> because of the following explanation it seems we have to bear with the
> current code.
>
> Suppose we have implemented your fix. say we call to functions x, y and z
> in that order. Say inside function x it set an error code status.
>
> Now we call y. Inside y() we neglect the error code and say we do even a
> database commit.
> Now we call to z(). At the beginning of z say we do an param check. If all
> parameters are good to go it should pass param checks. But it return an
> error (because of previously set error status in function x) even when all
> parameters are good. This is unfair for function z() and it is also
> misleading.
>
> So the best thing to do is to handle error status before calling any
> important function which has param checkings etc. For example in function
> y() we have even done a database commit neglecting the error status which
> is not good.
>
> thanks
> Damitha.
>
>
> Dimuthu Gamage wrote:
>
> Hi devs,
>>
>> Currently in AXIS2_PARAM_CHECK, if the param is null we set the status
>> code to AXIS2_FAILURE and if not null set the status code AXIS2_SUCCESSS
>>
>> #define AXIS2_PARAM_CHECK(error, object, error_return) \
>> if (!object) \
>> { \
>> AXIS2_ERROR_SET_ERROR_NUMBER(error,
>> AXIS2_ERROR_INVALID_NULL_PARAM); \
>> AXIS2_ERROR_SET_STATUS_CODE(error, AXIS2_FAILURE); \
>> return error_return; \
>> } \
>> else \
>> { \
>> AXIS2_ERROR_SET_STATUS_CODE(error, AXIS2_SUCCESS); \
>> }
>>
>>
>> My question is, is it ok to set the status code to AXIS2_SUCCESS,
>> Because
>> 1. If we are in happy path, the status code is already AXIS2_SUCCESS, we
>> don't need to explicitly set that.
>> 2. The macro can overwrite the status, That is if the status is already
>> set to failure by an early case, this macro will overwrite the status to
>> SUCCESS which is wrong.
>>
>> So my suggestion is to remove the else part from that macro?. Please let
>> me know your ideas, I may be wrong on this.
>>
>> Thanks
>> Dimuthu
>>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: axis-c-dev-unsubscribe@ws.apache.org
> For additional commands, e-mail: axis-c-dev-help@ws.apache.org
>
>
Re: Question about AXIS2_PARAM_CHECK
Posted by Damitha Kumarage <da...@gmail.com>.
Hi,
I had the same concern and tried a similar kind of fix few weeks back.
But because of the following explanation it seems we have to bear with
the current code.
Suppose we have implemented your fix. say we call to functions x, y and
z in that order. Say inside function x it set an error code status.
Now we call y. Inside y() we neglect the error code and say we do even a
database commit.
Now we call to z(). At the beginning of z say we do an param check. If
all parameters are good to go it should pass param checks. But it return
an error (because of previously set error status in function x) even
when all parameters are good. This is unfair for function z() and it is
also misleading.
So the best thing to do is to handle error status before calling any
important function which has param checkings etc. For example in
function y() we have even done a database commit neglecting the error
status which is not good.
thanks
Damitha.
Dimuthu Gamage wrote:
> Hi devs,
>
> Currently in AXIS2_PARAM_CHECK, if the param is null we set the status
> code to AXIS2_FAILURE and if not null set the status code AXIS2_SUCCESSS
>
> #define AXIS2_PARAM_CHECK(error, object, error_return) \
> if (!object) \
> { \
> AXIS2_ERROR_SET_ERROR_NUMBER(error,
> AXIS2_ERROR_INVALID_NULL_PARAM); \
> AXIS2_ERROR_SET_STATUS_CODE(error, AXIS2_FAILURE); \
> return error_return; \
> } \
> else \
> { \
> AXIS2_ERROR_SET_STATUS_CODE(error, AXIS2_SUCCESS); \
> }
>
>
> My question is, is it ok to set the status code to AXIS2_SUCCESS,
> Because
> 1. If we are in happy path, the status code is already AXIS2_SUCCESS,
> we don't need to explicitly set that.
> 2. The macro can overwrite the status, That is if the status is
> already set to failure by an early case, this macro will overwrite the
> status to SUCCESS which is wrong.
>
> So my suggestion is to remove the else part from that macro?. Please
> let me know your ideas, I may be wrong on this.
>
> Thanks
> Dimuthu
---------------------------------------------------------------------
To unsubscribe, e-mail: axis-c-dev-unsubscribe@ws.apache.org
For additional commands, e-mail: axis-c-dev-help@ws.apache.org