On Mon, 9 Aug 2010 11:32:20 -0500
Kent Yoder <[email protected]> wrote:

> 
> - fix compile warnings generated by -Wall
> - always return a non-zero error code to the command line on test error
> - use a macro to send output to stderr when appropriate
> - print file/line numbers for any failures
> 
> Signed-off-by: Kent Yoder <[email protected]>
> 

Thanks for the much-needed patches to sanitize the testcases!

A few comments below


>  {
>       CK_BYTE             data1[BIG_REQUEST];
>       CK_BYTE             data2[BIG_REQUEST];
> @@ -37,7 +37,7 @@ int do_EncryptAES_ECB(void)
>       CK_ULONG            user_pin_len;
>       CK_ULONG            i;
>       CK_ULONG            len1, len2, key_size = AES_KEY_SIZE_256;
> -     CK_RV               rc;
> +     CK_RV               rc, loc_rc;
>       CK_ATTRIBUTE        key_gen_tmpl[] = {
>               {CKA_VALUE_LEN, &key_size, sizeof(CK_ULONG) }
>       };
> @@ -49,7 +49,7 @@ int do_EncryptAES_ECB(void)
>       rc = funcs->C_OpenSession( slot_id, flags, NULL, NULL, &session );
>       if (rc != CKR_OK) {
>               show_error("   C_OpenSession #1", rc );

Are you planning in replacing the 'show error()'macro as well? Or at least 
build in terms
of the PRINT_ERR macro?


> +     rc = do_GetFunctionList();
> +     if (!rc) {
> +             PRINT_ERR("ERROR do_GetFunctionList() Failed , rc = 0x%0x\n", 
> rc); 
> +             return rc;
> +     }
> +     

trailing whitespace (see below)

> +     memset( &cinit_args, 0x0, sizeof(cinit_args) );
> +     cinit_args.flags = CKF_OS_LOCKING_OK;
> +
> +     // SAB Add calls to ALL functions before the C_Initialize gets hit
> +
> +     funcs->C_Initialize( &cinit_args );
> +
> +     {
> +             CK_SESSION_HANDLE  hsess = 0;
> +
> +             rc = funcs->C_GetFunctionStatus(hsess);
> +             if (rc  != CKR_FUNCTION_NOT_PARALLEL)  

Watch out for trailing spaces. I can fix them when applying, so don't worry
about them this time, but just fyi you can use 'git diff --check' or
'git apply --whitespace=show-all' to flag them.


> +                     return rc;
> +
> +             rc = funcs->C_CancelFunction(hsess);
> +             if (rc  != CKR_FUNCTION_NOT_PARALLEL)
> +                     return rc;
> +
> +     }
> +
> +     rv = aes_functions();
> +     /* make sure we return non-zero if rv is non-zero */
> +     return ((rv==0) || (rv % 256) ? rv : -1);

Any reason why mod 256? Can't we just 'return rv;'?


Thanks,

 -Klaus

-- 
Klaus Heinrich Kiwi | [email protected] | http://blog.klauskiwi.com
Open Source Security blog :     http://www.ratliff.net/blog
IBM Linux Technology Center :   http://www.ibm.com/linux/ltc

------------------------------------------------------------------------------
This SF.net email is sponsored by 

Make an app they can't live without
Enter the BlackBerry Developer Challenge
http://p.sf.net/sfu/RIM-dev2dev 
_______________________________________________
Opencryptoki-tech mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opencryptoki-tech

Reply via email to