[OPEN-ILS-DEV] PATCH: osrfConfig.c (default configuration)

[Mike Rylander]

On 5/26/07, Scott McKellar <mck9 at swbell.net> wrote:
> This patch tidies up the treatment of __osrfConfigDefault, which is a
> pointer to the default configuration, if there is one.
>
> 1. I declared this variable static so that it will be local to the
> source file in which it appears.  No other source files refer to it,
> nor should they.
>
> 2. I changed the name by removing the two leading underscores.
> Identifiers beginning with two leading underscores are reserved for
> use by the implementation.
>
> 3. In the osrfConfigSetDefaultConfig function I free the default
> configuration, if there is one, before installing the new one.  This
> change goes hand in hand with the next one:
>
> 4. In osrfConfigInit() I deleted the line that frees the prior
> default configuration.  This function has nothing otherwise to do
> with the default configuration.  It creates a new configuration,
> but does not install it as the default.  It has no business freeing
> the default if it is not going to replace it.
>
> At worst, this change could result in a memory leak, but only in
> an improbable sequence of events: the application (a) installs a
> default configuration, (b) creates a new configuration, and (c)
> does not install the new one as the default.  However I looked at
> all the code that currently calls osrfConfigInit() and satisfied
> myself that no such memory leak would occur.

And I could even see the utility in the above sequence.  Imagine an
app that uses the default configuration for core settings and then
loads other (possibly multiple simultaneous) configurations for use in
specific contexts.  Given that, I'd call this a small foot gun (foot
watergun?) at worst.

>
> If for some reason we really do need to free the default configuration
> at this point, we should at least set the pointer to NULL afterwards.
> It's asking for trouble to leave a pointer lying around pointing to
> something that doesn't exist any more.
>
> 5. I also fixed an obvious typo in one of the error messages.
>
> Scott McKellar
> http://home.swbell.net/mck9/aargh/
>

Thanks much, Scott!  It's in the queue.

--miker

> 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.
>


-- 
Mike Rylander