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

[Mike Rylander]

On Sat, Feb 23, 2008 at 5:17 PM, Scott McKellar <mck9 at swbell.net> wrote:
> This patch is mostly a performance tweak.

Applied, and thanks for the tips on time_t values.

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



-- 
Mike Rylander
 | VP, Research and Design
 | Equinox Software, Inc. / The Evergreen Experts
 | phone:  1-877-OPEN-ILS (673-6457)
 | email:  miker at esilibrary.com
 | web:  http://www.esilibrary.com