On Mon, 2012-12-03 at 15:34 +0100, Pavel Březina wrote:
On 12/02/2012 05:59 AM, Simo Sorce wrote:
> These functions allow handling of auth tokens in a completely opaque way,
> with clear semantics and accessor fucntions that guarantee consistency,
> proper access to data and error conditions.
> ---
/home/pbrezina/workspace/sssd/.git/rebase-apply/patch:229: new blank
line at EOF.
+
warning: 1 line adds whitespace errors.
Can we move enum sss_authtok_type to authtok.h?
Yes, I thought I did it, I had the same intention, will check if there
is a reason why I did not, otherwise I'll move.
> +size_t sss_authtok_get_size(struct sss_auth_token *tok)
> +{
> + switch (tok->type) {
> + case SSS_AUTHTOK_TYPE_PASSWORD:
> + case SSS_AUTHTOK_TYPE_CCFILE:
> + return tok->length;
> + default:
> + return 0;
> + }
> +}
Return 0 for SSS_AUTHTOK_TYPE_CCFILE and print error otherwise?
Also not using default branch for enum would be nice, because we'll get
compiler error if we add new enum value and not handle it in switch.
Oh, yes, I actually liked they were enum's exactly because then I could
avoid 'default' ... thanks for catching this.
> +
> +uint8_t *sss_authtok_get_data(struct sss_auth_token *tok)
> +{
> + return (void *)tok->data;
> +}
Why are you typecasting to void* when you are returning uint8_t*? And
tok->data is already uint8_t*.
bug, initially the interface was returning void *, then I change it,
thanks for catching.
> +static errno_t sss_authtok_set_string(TALLOC_CTX *mem_ctx,
> + struct sss_auth_token *tok,
> + enum sss_authtok_type type,
> + const char *context_name,
> + const char *str, size_t len)
> +{
> + size_t size;
> +
> + if (len == 0) {
> + len = strlen(str);
> + } else {
> + while (len > 0 && str[len - 1] == '\0') len--;
> + }
> +
> + if (len == 0) {
You should check len <= 0, because if negative len was passed in
parameter list, it will get here unchanged.
size_t is unsigned, it can't represent negative numbers :-)
> + /* we do not allow zero length ttyped tokens */
^ typo
will fix
> +/* Auth token structure,
> + * please never use directly.
> + * Use ss_authtok_* accesor functions instead
^typo
as well.
Thanks,
Simo.
--
Simo Sorce * Red Hat, Inc * New York