Sorry, I really messed up my Subject header.
This comment was for [PATCH 2/8] [CRYPTO] aead: Add support for 
multiple template parameters in the patchset.

The second patch in patch set, not the eighth.

Joy

On Thu, 2007-09-27 at 17:25 -0500, Joy Latten wrote:
> My apologies for such late notice, but I observed this only recently
> and wanted to point it out. 
> 
> 
>       if (!try_module_get(THIS_MODULE))
>               goto err;
> @@ -101,33 +106,73 @@ static int cryptomgr_schedule_probe(struct 
> crypto_larval *larval)
>  
>       memcpy(param->template, name, len);
>  
> -     name = p + 1;
> -     len = 0;
> -     for (p = name; *p; p++) {
> -             for (; isalnum(*p) || *p == '-' || *p == '_' || *p == '('; p++)
> -                     ;
> +     i = 0;
> +     for (;;) {
> +             int notnum = 0;
>  
> -             if (*p != ')')
> -                     goto err_free_param;
> +             name = ++p;
> +             len = 0;
> +
> +             for (; isalnum(*p) || *p == '-' || *p == '_'; p++)
> +                     notnum |= !isdigit(*p);
> +
> +             if (*p == '(') {
> +                     int recursion = 0;
> +
> +                     for (;;) {
> +                             if (!*++p)
> +                                     goto err_free_param;
> +                             if (*p == '(')
> +                                     recursion++;
> +                             else if (*p == ')' && !recursion--)
> 
> Shouldn't p be incremented after this check? Otherwise, it will 
> still be pointing to ')' after breaking from this for-loop and we will
> end up copying "hmac(sha1" instead of "hmac(sha1)", right?
> Also, I think we will prematurely break from top for-loop 
> because of check, "if (*p == ')') break;" which is done further down...
> 
> +                                     break;
> +                     }
> +
> +                     notnum = 1;
> +             }
>  
>               len = p - name;
> +             if (!len)
> +                     goto err_free_param;
> +
> +             if (notnum) {
> +                     param->attrs[i].alg.attr.rta_len =
> +                             sizeof(param->attrs[i].alg);
> +                     param->attrs[i].alg.attr.rta_type = CRYPTOA_ALG;
> +                     memcpy(param->attrs[i].alg.data.name, name, len);
> +             } else {
> +                     param->attrs[i].nu32.attr.rta_len =
> +                             sizeof(param->attrs[i].nu32);
> +                     param->attrs[i].nu32.attr.rta_type = CRYPTOA_U32;
> +                     param->attrs[i].nu32.data.num =
> +                             simple_strtol(name, NULL, 0);
> +             }
> +
> +             param->tb[i + 1] = &param->attrs[i].attr;
> +             i++;
> +
> +             if (WARN_ON(i >= CRYPTO_MAX_ATTRS))
> +                     goto err_free_param;
> +
> +             if (*p == ')')
> +                     break;
> +
> +             if (*p != ',')
> +                     goto err_free_param;
>       }
> 
> 
> Regards,
> Joy
> -
> To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
> the body of a message to [EMAIL PROTECTED]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
-
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to