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)
> @@ -21,6 +21,7 @@
> #define APR_WANT_STRFUNC
> #include <apr.h>
> #include <apr_want.h>
> +#include <apr_fnmatch.h>
>
> #include <serf.h>
> #include <serf_bucket_types.h>
> @@ -41,6 +42,145 @@
> #define XML_STATUS_ERROR 0
> #endif
>
> +static const apr_uint32_t serf_failure_map[][2] =
> +{
> + { SERF_SSL_CERT_NOTYETVALID, SVN_AUTH_SSL_NOTYETVALID },
> + { SERF_SSL_CERT_EXPIRED, SVN_AUTH_SSL_EXPIRED },
> + { SERF_SSL_CERT_SELF_SIGNED, SVN_AUTH_SSL_UNKNOWNCA },
> + { SERF_SSL_CERT_UNKNOWNCA, SVN_AUTH_SSL_UNKNOWNCA }
> +};
> +
> +/* Convert neon's SSL failure mask to our own failure mask. */
>
> +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.
Some other functions introduced in this commit do not even have doc
strings. Sure, when the behavior of a function is *completely* obvious
from its name and arguments, I agree that the doc string can be
dispensed with (but even then it's better to write it, and easy to do so
because it's obvious.) Anyway, such functions are are quite rare.
> +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.)
> +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?
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.
-Karl
---------------------------------------------------------------------
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 17:44:45 CET