[
https://issues.apache.org/jira/browse/AXIS2C-783?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12545573
]
Bill Mitchell commented on AXIS2C-783:
--------------------------------------
Dinesh, to examine your change I downloaded the current development sources.
First, your change does appear to resolve my comment on item(1) in the
processing. Thank you; that's great. This still leaves the more important
issues, that the error code you so nicely communicate is then bashed by the
code in stub.c at point (4) in the processing, after which we fall into the seg
fault on the null pointer reference at point (5) in the processing.
I did like the additional logging messages I see. I worry that the code that
generates this in axis2_build_client_conf_ctx() also introduces a memory leak.
The code fragment:
conf = axis2_dep_engine_load_client(dep_engine, env, axis2_home);
if (!conf)
{
AXIS2_LOG_ERROR(env->log, AXIS2_LOG_SI,
"dep engine load failed. conf value is NULL");
return NULL;
}
axis2_conf_set_dep_engine(conf, env, dep_engine);
should free the dep_engine on the error path:
if (!conf)
{
AXIS2_LOG_ERROR(env->log, AXIS2_LOG_SI,
"dep engine load failed. conf value is NULL");
axis2_dep_engine_free(dep_engine, env);
return NULL;
}
In stepping through the code, I came across a new issue in the code. In
conf_init.c, the function axis2_build_client_conf_ctx() now accepts an home
directory that points to the axis2.xml file itself or the parent directory.
Unfortunately, the code that checks if the file ends in axis2.xml examines the
trailing 9 characters of the home variable, even if the string is shorter than
9. Even on platforms where this does not generate a seg fault, it is really
bad form.
In the code fragment:
status = axutil_file_handler_access (axis2_home, AXIS2_R_OK);
if (status == AXIS2_SUCCESS)
{
len = strlen (axis2_home);
if (!strcmp ((axis2_home + (len - 9)), "axis2.xml"))
{
dep_engine = axis2_dep_engine_create_with_axis2_xml (env,
axis2_home);
}
else
{
dep_engine = axis2_dep_engine_create(env);
}
}
else
the if test should be:
if ((len >= 9) &&
!strcmp ((axis2_home + (len - 9)), "axis2.xml"))
By the way, independent of your changes, in order to test your changes I had to
repair a compilation failure building Axis2C in
src\core\clientapi\svc_client.c. In axis2_svc_client_set_policy_from_om(), the
AXIS2_PARM_CHECK needs to follow the declaration:
axis2_svc_client_set_policy_from_om(
axis2_svc_client_t * svc_client,
const axutil_env_t * env,
axiom_node_t * root_node)
{
AXIS2_PARAM_CHECK (env->error, svc_client, AXIS2_FAILURE);
neethi_policy_t *neethi_policy = NULL;
neethi_policy = neethi_util_create_policy_from_om(env, root_node);
if (neethi_policy)
{
return axis2_svc_client_set_policy(svc_client, env, neethi_policy);
}
else
{
return AXIS2_FAILURE;
}
}
should read:
...
neethi_policy_t *neethi_policy = NULL;
AXIS2_PARAM_CHECK (env->error, svc_client, AXIS2_FAILURE);
...
Also, to try the test completely from the beginning, I downloaded as well the
snapshot of the WSDL2C in the Axis2 Java to generate the stubs from the
beginning. For some reason that I have not isolated, the WSDL2C now generates
no code whatsoever for my WSDL file, even when I specify none for the
databindings.
> Client seg faults if AXIS2C_HOME variable not set
> -------------------------------------------------
>
> Key: AXIS2C-783
> URL: https://issues.apache.org/jira/browse/AXIS2C-783
> Project: Axis2-C
> Issue Type: Bug
> Components: code generation
> Affects Versions: 1.1.0
> Environment: Window XP, Visual Studio 2005
> Reporter: Bill Mitchell
>
> If the AXIS2C_HOME variable is not set, or if the client application code
> sets the ClientHome argument to a directory that does not containt axis2.xml
> and related files, the client application fails with a segmentation fault.
> Investigating this uncovered several issues, best described by highlighting
> the relevant actions in the code in chronological order:
> (1) axis2_dep_engine_check_client_home() in depengine.c detects the problem
> and stores an error code, AXIS2_ERROR_CONFIG_NOT_FOUND (18). But processing
> at this point is allowed to continue.
> (2) axis2_conf_builder_create_with_file_and_dep_engine_and_conf(), in
> particular axis2_desc_builder_create_with_file_and_dep_engine(), then changes
> the status_code in the environment error structure back to success. This is
> a side-effect of invoking the AXIS2_PARM_CHECK macro.
> (3) In axis2_desc_builder_build_om(), the call to
> axiom_xml_reader_create_for_file() returns an error, but the error number has
> now been replaced
> with AXIS2_ERROR_CREATING_XML_STREAM_READER(141).
> Axis2_desc_builder_build_om() then replaces this error number with the same
> value, just to
> make sure it is well and truly changed.
> (4) In stub.c, axis2_stub_create_with_endpoint_ref_and_client_home() replaces
> the error 141 with a truly misleading AXIS2_ERROR_NO_MEMORY(2).
> (5) In the generated stub code to create the service,
> axis2_stub_servicename_create() ignores the fact that the returned stub
> pointer is zero and goes ahead and calls
> axis2_stub_servicename_populate_services() anyway, where the code dies
> dereferencing the zero pointer.
> Recommendations:
> (5) In the generated stub code for axis2_stub_servicename_create(), the
> fragment
> stub = axis2_stub_create_with_endpoint_ref_and_client_home ( env,
> endpoint_ref, client_home );
> axis2_stub_servicename_populate_services( stub, env );
> return stub;
> should read:
> stub = axis2_stub_create_with_endpoint_ref_and_client_home ( env,
> endpoint_ref, client_home );
> if (stub)
> {
> axis2_stub_servicename_populate_services( stub, env );
> }
> return stub;
> (4) In stub.c, the procedure
> axis2_stub_create_with_endpoint_ref_and_client_home() really needs to let the
> underlying error number be returned. In the fragment:
> /* create service_client*/
> stub->svc_client = axis2_svc_client_create(env , client_home);
> if (!stub->svc_client)
> {
> axis2_stub_free(stub, env);
> AXIS2_ERROR_SET(env->error, AXIS2_ERROR_NO_MEMORY, AXIS2_FAILURE);
> return NULL;
> }
> the AXIS2_ERROR_SET invocation should be removed.
> (1) Where the client has passed a clienthome parameter, but it does not point
> to a valid Axis2C home directory structure, I prefer the first error code
> AXIS2_ERROR_CONFIG_NOT_FOUND to the second
> AXIS2_ERROR_CREATING_XML_STREAM_READER. So I suggest that
> axis2_dep_engine_load_client() in depengine.c be changed to treat this as an
> immediate error instead of letting processing continue. The fragment:
> if (client_home && 0 != axutil_strcmp("", client_home))
> {
> status = axis2_dep_engine_check_client_home(dep_engine, env,
> client_home);
> if (AXIS2_SUCCESS == status)
> {
> is_repos_exist = AXIS2_TRUE;
> }
> }
> else
> could be changed to:
> if (client_home && 0 != axutil_strcmp("", client_home))
> {
> status = axis2_dep_engine_check_client_home(dep_engine, env,
> client_home);
> if (AXIS2_SUCCESS != status)
> {
> return NULL;
> }
> is_repos_exist = AXIS2_TRUE;
> }
> else
> Of course, there may be a good reason to want to continue processing that I
> have not uncovered. In this case, you could instead do something to resolve
> item(2), such that the original error code could be returned instead of
> overlaying it as happens at point (3).
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]