[OPEN-ILS-DEV] PATCH: osrf_system.[ch]

[Scott McKellar]

Nothing major here -- some tidying up, and some minor changes in
functionality.

1. I turned __osrfGlobalTransportClient into a static variable, and
removed the leading underscores from its name.  It is not referenced
outside of this source file, and shouldn't be.

2. I turned _osrfSystemInitCache() into a static function, and moved
its declaration from the header file to the implementation file.  No
other source file refers to it.  I didn't remove the leading 
underscore because a singe leading underscore followed by a lower case 
letter is okay for an identifier at file scope.

3. I turned __osrfSystemSignalHandler() into a static function as 
well, and removed the leading underscores,  It isn't referenced 
elsewhere.  As I have noted in another thread, this signal handler
should be eliminated anyway, but I went ahead and made it static as 
long as I was in this file for other reasons.

4. I was tempted to make osrf_system_disconnect_client() static too,
but stayed my hand.  It is not referenced in any other source file
apart from the header, but it kinda sorta looks like maybe it was
intended to be, potentially, someday.

5. For functions that take no parameters, I inserted "void" into the
formal parameter list.  This change requires a little explanation.

The following is a function declaration:

    int osrf_foo();

It tells the compiler the name and return type of the function, but
it says nothing about the number or types of parameters.  If you
later code:

    int n = osrf_foo( stderr, cfg, "Random message\n", 42 );

...the compiler has to let you, because it has no way of knowing
whether this function takes any parameters.

(Of course it *might* give you a warning message anyway, if it 
wants to.  The compiler is entitled to issue a diagnostic about
absolutely anything, including the color of your socks, but it
still has to compile the code if the code follows the rules.)

The following, with "void" as the formal parameter list, is a 
function prototype:

    int osrf_foo( void );

With such a prototype in scope, the compiler knows that you 
shouldn't pass parameters to this function, and it must issue a
diagnostic if you try.

Moral: always use "void" as the formal parameter list for a function
that takes no parameters, so that the compiler can protect you from
yourself.  (I am describing C89.  For all I know the rules may be 
different for C99, but I doubt it.  Also the "void" is not necessary
in C++, but it does no harm if present.)

6. In osrf_system_bootstrap_client_resc() I no longer call
osrfConfigCleanup() before calling osrfConfigInit().  Given the patch
that I submitted earlier today, which has already been applied, this
call is now redundant.  We will discard the previous default
configuration, if any, when we call osrfConfigSetDefault().  See also
the next item...

6. In osrf_system_bootstrap_client_resc() I check the value returned
by osrfConfigInit(), and exit immediately if it is NULL.  I don't
issue a message because osrfConfigInit() will have already done so.

If we don't have a configuration, then there's no point in continuing
further, possibly issuing additional error messages that would just
be a distraction from the real problem.

There is also a more subtle difference in behavior.  In the old code
we would discard the previous default configuration, if any, 
unconditionally, before even trying to create a new one.  With my 
patch, if we are unable to create a new configuration, we keep the 
old one in place.

The alternative is to call osrfConfigCleanup() before returning.

For today's applications I doubt that it makes any significant
difference either way.  I chose to leave the old configuration in
place because of the following rule of thumb: if a function fails,
it should fail without leaving any side effects.  In this case, let
the calling code decide whether it wants to keep the old 
configuration or not.

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_system_c_3.patch
Type: text/x-patch
Size: 4432 bytes
Desc: 1491318739-osrf_system_c_3.patch
Url : http://list.georgialibraries.org/pipermail/open-ils-dev/attachments/20070526/1d87791f/osrf_system_c_3.bin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: osrf_system_h_1.patch
Type: text/x-patch
Size: 1084 bytes
Desc: 4075430451-osrf_system_h_1.patch
Url : http://list.georgialibraries.org/pipermail/open-ils-dev/attachments/20070526/1d87791f/osrf_system_h_1.bin