Ajay & Ashok

Ajay's and Ashok's comments for

 Last update: 2008-06-05
Batch 0

Ajay's review comments :
~----------------------

ns_sldap.h :
~------------

line 495  * NSBEC:

I think "_" is missed i.e should be : NS_BEC

Also on lines 533 & 539

Tomas: All NSBEC, NS_BEC etc. references were removed from the code, comments 
       etc.

ldaplist.c
~----------

line 340      standalone_cfg.SA_AUTH = authmech == NULL ? NULL : &auth;

Just a comment : for readability purpose

standalone_cfg.SA_AUTH = ( authmech == NULL ) ? NULL : &auth;

Tomas: Done.

lines 393-397 comments should say only "SASL/GSSAPI auth method will be
used"

Tomas: The comment has been reformatted/reformulated somewhat.

ldapclient.c :
~--------------

1457 /*      __ns_ldap_setServer(TRUE); */

one line comment is required i.e why is it not required now or this
should be deleted.

Tomas: Dead code removed.

Ashok review comment's :
~------------------------

* ns_config.c:
            line 795: Usage of free() before assigning NULL to
            current_config would safe.

mc> free(ptr) is done at 803 after all the resources are freed.
mc> Before that we can't free the current_config.

standalone.h:  Function separatePort():
            We are not taking care of syntax errors like that of
            '...]...[....'

Tomas: Right. We don't at that place. If you can come up with real case
       of making this fail over because of syntax like that, please file
       CR and let me know.

ns_standalone.c: Function get_db():
           line 228, 242, 311, 317: It would safe if we can check for
           the end of buffer in the while loop when checking for space.

Tomas: I believe the function is safe w.r.t. end of string. Some of it
       was even rewritten and special care for end-of-string situation
       has been taken.

           Typo in line 284:  longer then -> longer than

Tomas: Fixed.

           lines 290: Most of the customers complain that certain
           application has exited without printing any log/error
           message. Introducing more exit messages would help understand
           why the application has exited. In this line we would like to
           have memory allocation error message here.

                  line 406: Return value of nss_search(). What if the
                  res is NSS_NOTFOUND? Function __s_api_ip2hostname():
                  Do we need to check for syntax errors like that of
                  [[]] or ...]...[... User could be crazy guy like me
                  :-)

           lines where inet_pton() is used: what if inet_pton() fails
           and returns -1. Don't we need to check for failure?

Tomas: We have another project going on dealing with logging. I'll leave
      the log issues for later to be addressed by the other project.