[SSSD] [PATCH 3/4] Add authtok utility functions.
Simo Sorce
simo at redhat.com
Mon Dec 3 20:10:53 UTC 2012
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
More information about the sssd-devel
mailing list