[OPEN-ILS-DEV] PATCH: socket_bundle.c (latent bugs)

[Mike Rylander]

On Sat, Mar 1, 2008 at 5:38 PM, Scott McKellar <mck9 at swbell.net> wrote:
> 1. In socket_open_unix_server() and socket_open_unix_client(), I added
>  checks to make sure that the path parameter doesn't point to aomething
>  too big to fit into the receiving buffer of the struct sockaddr_un.
>
>  2. In _socket_handle_client_data() I add a terminal nul to the data
>  received by recv().
>

Applied ...

>  -----------
>
>  At one time we were using memset() to pre-fill the buffer with nuls.
>  This was an inefficient but effective way to ensure the presence of a
>  terminal nul.
>
>  At some point the memset() was replaced by osrf_clearbuf(), a macro
>  that either does the original memset() or fills the buffer with
>  exclamation points plus a terminal nul, depending on whether NDEBUG is
>  #defined.  In the latter case, the result will be that the input
>  message will be padded with exclamation points to a length of 1023.
>
>  Apparently this hasn't happened, or nobody noticed.
>
>  I suspect it never happened in testing, but may happen in production,
>  because I suspect that the conditional compilation is working
>  backwards from what was intended.  We define orsr_clearbuf() thus:
>
>  #ifndef NDEBUG
>  // The original ... replace with noop once no more errors occur in
>  NDEBUG mode
>  #define osrf_clearbuf( s, n ) memset( s, 0, n )
>  #else
>  #define osrf_clearbuf( s, n ) \
>         do { \
>                 char * clearbuf_temp_s = (s); \
>                 size_t clearbuf_temp_n = (n); \
>                 memset( clearbuf_temp_s, '!', clearbuf_temp_n ); \
>                 clearbuf_temp_s[ clearbuf_temp_n - 1 ] = '\0'; \
>         } while( 0 )
>  #endif
>
>  The normal convention is that NDEBUG means "debugging code is turned
>  off."  In particular, NDEBUG turns the standard assert() macro into
>  a no-op.
>
>  By that convention, the above macro uses the exclamation points
>  only when debugging is turned off, i.e. only in production code.
>
>  Surely that's not what was intended.  I believe we should be using
>  "#ifdef" rather than "#ifndef".  Of course it is also possible that
>  the makefiles use NDEBUG in the opposite of the usual sense.  Such
>  a perverse practice would in principle be harmless as long as we
>  don't use the assert() macro -- and currently we don't.
>

You are correct about the semantics of NDEBUG have been
abused/reversed.  NDEBUG is currently set to turn on debugging, though
obviously (now, anyway) that's incorrect.

In addition to this patch, I've reversed the semantics of NDEBUG and
make NDEBUG the default via the makefile.  To turn on debugging code,
supply DEBUG=1 as an environment variable during the build, for
example:

  $ DEBUG=1 make clean all

Thanks, Scott!

>  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