[OPEN-ILS-DEV] PATCH: osrf_settings.c

[Scott McKellar]

This patch chips away at the "config pointer is NULL" problem that
some people still report from time to time.

1. In an earlier patch to osrf_settings_host_value_object(), we
contrived to abort the program deliberately instead of crashing with
a segmentation violation.  The current patch applies the same 
treatment to the similar function osrf_settings_host_value(),
although I don't know that anyone has ever reported a problem at
this spot.  I rearranged the error message a bit so that it would
be distinct from the other one.

2. If the config pointer is null, it's because osrf_settings_retrieve()
was unable to populate it, or was never called.  There are two ways
it might fail: it might not get an osrf_message from 
osrf_app_session_request_recv(), or the osrf_message it gets might 
not have the _result_content member populated.  I added a couple of
log messages to distinguish between these possibilities. Maybe they 
will make diagnosis easier.

Incidentally the wording of my proposed messages could probably be
improved.  I don't know enough to make them meaningful to someone 
who isn't staring at the source code.

3. The latter change also plugs a small memory leak.  If we get an
osrf_message but it has no _result_content, the present code neglects
to free the osrf_message.

----------

One other observation:

The config pointer in question is defined at global scope in
osrf_settings.c.  However it is not declared in the corresponding
header file.  I strongly suspect that it is intended to be 
accessed only from within the source file where it resides.

In that case it should be declared static.

However it is conceivable that some other header declares this
pointer, or that some other module declares it outside of a header.
I doubt it, but it's possible.

Unfortunately if I grep for "config" I'll get a ton of spurious
hits, and I'm too lazy to go through them to verify that they all
refer to something else.

As a result, this patch doesn't change the declaration of the config
pointer.  Mike or Bill can decide much more readily than I can
whether the config pointer deserves to be static.

Scott McKellar
http://home.swbell.net/mck9/aargh/

------------------------

Developer's Certificate of Origin 1.1 By making a contribution to
this project, I certify that:

(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license indicated
in the file; or

(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source license
and I have the right under that license to submit that work with
modifications, whether created in whole or in part by me, under the
same open source license (unless I am permitted to submit under a
different license), as indicated in the file; or

(c) The contribution was provided directly to me by some other person
who certified (a), (b) or (c) and I have not modified it; and

(d) In the case of each of (a), (b), or (c), I understand and agree
that this project and the contribution are public and that a record
of the contribution (including all personal information I submit
with it, including my sign-off) is maintained indefinitely and may
be redistributed consistent with this project or the open source
license indicated in the file.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: osrf_settings_c_2.patch
Type: text/x-patch
Size: 1490 bytes
Desc: 4032675458-osrf_settings_c_2.patch
Url : http://list.georgialibraries.org/pipermail/open-ils-dev/attachments/20070602/28a553b8/osrf_settings_c_2.bin