Karl Fogel wrote:
> 2008-03-17 02:27 lgo_at_tigris.org <lgo_at_tigris.org> napisał(a):
>> --- trunk/subversion/libsvn_ra_serf/util.c (r29938)
>> +++ trunk/subversion/libsvn_ra_serf/util.c (r29939)
..
>> +static apr_uint32_t
>> +ssl_convert_serf_failures(int failures)
>> +{
>> + apr_uint32_t svn_failures = 0;
>> + apr_size_t i;
>> +
>> + for (i = 0; i < sizeof(serf_failure_map) / (2 * sizeof(apr_uint32_t)); ++i)
>> + {
>> + if (failures & serf_failure_map[i][0])
>> + {
>> + svn_failures |= serf_failure_map[i][1];
>> + failures &= ~serf_failure_map[i][0];
>> + }
>> + }
>> +
>> + /* Map any remaining failure bits to our OTHER bit. */
>> + if (failures)
>> + {
>> + svn_failures |= SVN_AUTH_SSL_OTHER;
>> + }
>> +
>> + return svn_failures;
>> +}
>
> It took me a while to figure out that the line
>
> failures &= ~serf_failure_map[i][0];
>
> was to remove the current failure from the original mask, so that at the
> end we could see if any are left over, and if so set OTHER. I updated
> the doc string in r30087 to describe the fall through into OTHER.
>
Thanks for that.
>
>> +static char *
>> +convert_organisation_to_str(apr_hash_t *org, apr_pool_t *pool)
>> +{
>> + char *str;
>> +
>> + str = apr_psprintf(pool, "%s, %s, %s, %s, %s (%s)",
>> + (char*)apr_hash_get(org, "OU", APR_HASH_KEY_STRING),
>> + (char*)apr_hash_get(org, "O", APR_HASH_KEY_STRING),
>> + (char*)apr_hash_get(org, "L", APR_HASH_KEY_STRING),
>> + (char*)apr_hash_get(org, "ST", APR_HASH_KEY_STRING),
>> + (char*)apr_hash_get(org, "C", APR_HASH_KEY_STRING),
>> + (char*)apr_hash_get(org, "E", APR_HASH_KEY_STRING));
>> +
>> + return str;
>> +}
>
> I gave this one a doc string too, though I admit the need was much less.
> (I also removed the holder variable.)
>
I've updated it a bit in r30092.
>> +static apr_status_t
>> +ssl_server_cert(void *baton, int failures,
>> + const serf_ssl_certificate_t *cert)
>> +{
>
> I have no idea looking at this what it does, so I didn't document it.
> Could you?
>
It's a callback function, implementing an interface documented in the
serf API. Docstring added in r30092.
> I know I harp on this doc string thing a lot, but most of our code is
> going to be read more than it is written. Doc strings save us time over
> the long run, and prevent distraction.
No need to convince me, but thank you for the review and friendly reminder!
Lieven
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-03-27 19:58:51 CET