[OPEN-ILS-DEV] PATCH: oils_cstore.c (performance)

[Scott McKellar]

This patch is mostly a performance tweak.

1. I replaced all instances of "jsonParseString( "[]" )" with
"jsonNewObjectType(JSON_ARRAY)", which produces the same result
with less work.

2. Likewise I replaced all instances of "jsonParseString( "{}" )"
with "jsonNewObjectType(JSON_HASH)".

3. In two spots I eliminated a memset() applied to _tmp_dt, a variable
of type time_t.

4. In several calls to strftime() I used the sizeof operator to
replace hard-coded buffer lengths.

-----------

If you want to initialize a variable of type time_t, set it to zero.
Don't use memset() on it.  Usually the result will be the same,
but it is possible (however unlikely) for a time_t to be implemented
as a floating point type rather than by an integral type.  In that
case setting all bits to zero is not likely to do anything useful.

In this case an assignment to _tmp_dt immediately followed the
memset(), which was therefore completely useless.

Nearby there are other calls to memset() that are almost as pointless,
but I left them alone.

--------

The use of sizeof was stimulated by my looking at the RATS report
that Dan Scott posted in November, pointing out potential security
risks.

RATS apparently got its knickers in a knot over the existence of
a couple of fixed length buffers allocated on the stack.  We weren't
doing anything unsafe with these buffers.  We pass them to memset()
and strftime(), both of which promise not to overflow the buffer
based on a size passed as a parameter.  I added the sizeof because
it's good practice, not because the existing code was buggy or unsafe.

Inference: RATS makes no attempt to determine that a bug or security
risk actually exists.  It merely looks for certain syntactic
constructs that are capable of abuse and misuse.

Corollary: It is neither necessary nor desirable to eliminate all
RATS warnings.  In this case, doing so would require that we allocate
the buffers dynamically -- thereby incurring more overhead, and 
creating opportunities for memory leaks or other bugs, without 
enhancing actual security by so much as a whisker.

I don't mean to suggest that the RATS reports are of no value.  I
just don't want anyone to invest them with more significance than
they deserve.

It might be nice to get Coverity to scan our code...

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

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: oils_cstore_c_5.patch
Type: text/x-patch
Size: 9500 bytes
Desc: 1530489508-oils_cstore_c_5.patch
Url : http://list.georgialibraries.org/pipermail/open-ils-dev/attachments/20080223/b70c4d04/oils_cstore_c_5.bin