ldap unreleasable

["William A. Rowe Jr." <wr...@rowe-clan.net>]

/* LDAP cache state information */
typedef struct util_ldap_state_t {
...
    int connectionPoolTTL;
} util_ldap_state_t;


I'm continue to grow more worried that the state of ldap in httpd
and in apr enjoys very little granularity, oversight, or quality...

 1. Hungarian?  Forgot to eat breakfast that day?  Out of bounds
    per httpd style rules.

 2. int?  Really?  This is assigned an apr_interval_time_t in its
    config code.

Please review and fix the style violations, and ensure that timeouts
are doing what they were meant to do.

Re: ldap unreleasable

["William A. Rowe Jr." <wr...@rowe-clan.net>]

On 3/28/2011 6:18 PM, Graham Leggett wrote:
> 
> Looks like mod_ssl needs some similar cleanup, there are still style violations in there,
> not limited to the examples below. Now would be a good time to overhaul style.

Agreed; it's much harder (and less justifiable) to make these sorts
of changes mid-maintenance after a 2.4.0 tag, especially since they
are harder to follow/ignore than whitespace changes.

+1 to whomever wants to do the very significant work of un-Hungarianizing
mod_ssl at this juncture.

The reason I never did this is because SSL uses some Hungarian notation
itself, and at the interface boundary it's less worrying.  But you are
pointing out httpd'isms and apr'isms which shouldn't have been expressed
that way.

FWIW I come from that background myself :)  But my examples in httpd
are limited to the places in mod_isapi where we intersect with Win32
constructs, so it actually makes the code more legible to know 'that
data' is over on the isapi side of the world, and this other data is
rooted in and managed by httpd.

Re: ldap unreleasable

[Graham Leggett <mi...@sharp.fm>]

On 29 Mar 2011, at 12:13 AM, William A. Rowe Jr. wrote:

> /* LDAP cache state information */
> typedef struct util_ldap_state_t {
> ...
>    int connectionPoolTTL;
> } util_ldap_state_t;
>
>
> I'm continue to grow more worried that the state of ldap in httpd
> and in apr enjoys very little granularity, oversight, or quality...
>
> 1. Hungarian?  Forgot to eat breakfast that day?  Out of bounds
>    per httpd style rules.
>
> 2. int?  Really?  This is assigned an apr_interval_time_t in its
>    config code.
>
> Please review and fix the style violations, and ensure that timeouts
> are doing what they were meant to do.

Please be aware that the person who donated this code to us isn't an  
ASF person, and donated the code in good faith. If it doesn't fit our  
style guide, we should fix it, and show respect for their original  
contribution.

Looks like mod_ssl needs some similar cleanup, there are still style  
violations in there, not limited to the examples below. Now would be a  
good time to overhaul style.

typedef struct {
   ...
} SSLConnRec;

typedef struct {
     pid_t           pid;
     apr_pool_t     *pPool;
     BOOL            bFixed;

     apr_global_mutex_t   *pMutex;
     apr_array_header_t   *aRandSeed;
     apr_hash_t     *tVHostKeys;
     void           *pTmpKeys[SSL_TMP_KEY_MAX];

     apr_hash_t     *tPublicCert;
     apr_hash_t     *tPrivateKey;

     const char     *szCryptoDevice;

     struct {
         void *pV1, *pV2, *pV3, *pV4, *pV5, *pV6, *pV7, *pV8, *pV9,  
*pV10;
     } rCtx;
} SSLModConfigRec;

Regards,
Graham
--

Re: ldap unreleasable

[Eric Covener <co...@gmail.com>]

On Mon, Mar 28, 2011 at 6:13 PM, William A. Rowe Jr.
<wr...@rowe-clan.net> wrote:
> /* LDAP cache state information */
> typedef struct util_ldap_state_t {
> ...
>    int connectionPoolTTL;
> } util_ldap_state_t;
>
>
> I'm continue to grow more worried that the state of ldap in httpd
> and in apr enjoys very little granularity, oversight, or quality...
>
>  1. Hungarian?  Forgot to eat breakfast that day?  Out of bounds
>    per httpd style rules.
>
>  2. int?  Really?  This is assigned an apr_interval_time_t in its
>    config code.
>
> Please review and fix the style violations, and ensure that timeouts
> are doing what they were meant to do.
>

Thanks for the review, blockers above in r1086432 (and bugfix in r1086433)

-- 
Eric Covener
covener@gmail.com