On Sun, Dec 08, 2013 at 01:36:15AM +0545, Pallavi Jha wrote:
> For  your comment
>
> I think the checks below are not necessary, are they? You already test
> sss_authtok_set with sss_authtok_get_ccfile and also
> sss_authtok_set_ccfile with sss_authtok_get_ccfile.
>
> Maybe I'm not seeing something, but why another sss_authtok_set and
> sss_authtok_get_ccfile?
>
> > +    ret = sss_authtok_set(ts->authtoken, type, data, 0);
> > +
> > +    assert_int_equal(ret, EOK);
> > +    assert_int_equal(type, sss_authtok_get_type(ts->authtoken));
> > +    assert_int_equal(len, sss_authtok_get_size(ts->authtoken));
> > +    assert_string_equal(data, sss_authtok_get_data(ts->authtoken));
> > +
> > +    ret = sss_authtok_get_ccfile(ts->
> authtoken, &pwd, &ret_len);
> > +
> > +    assert_int_equal(ret, EOK);
> > +    assert_string_equal(data, pwd);
> > +    assert_int_equal(len - 1, ret_len);
> > +}
> > +
> > +/* Test when type has value SSS_AUTHTOK_TYPE_EMPTY */
>
>
> Here I have tested what will happen when length is zero. Previously it was
> not tested. I have made the changes. Kindly review the patch.

Ah, thanks for explanation.

>
> With regards
> Pallavi Kumari Jha
> Computer Science Engineering

There is still some changes that need to be done, the current version of
the patch crashes for me. See below:

> +/* @test_authtok_type_x : tests following functions for different value of type
> + * sss_authtok_set
> + * sss_authtok_get_type
> + * sss_authtok_get_size
> + * sss_authtok_get_data
> + * sss_authtok_get_password
> + * sss_authtok_get_ccfile
> + *

> + * @test_authtok_type_password : type => SSS_AUTHTOK_TYPE_PASSWORD
> + * @test_authtok_type_ccfile   : type => SSS_AUTHTOK_TYPE_CCFILE
> + * @test_authtok_type_empty    : type => SSS_AUTHTOK_TYPE_EMPTY
> + */
> +

> +/* Test when type has value SSS_AUTHTOK_TYPE_PASSWORD */
> +static void test_sss_authtok_password(void **state)
> +{
> +    size_t len;
> +    errno_t ret;
> +    const char *data;
> +    size_t ret_len;
> +    const char *pwd;
> +    struct test_state *ts;
> +    enum sss_authtok_type type;
> +

> +    data = talloc_strdup(ts, "password");

Here you use "ts" before initializing it. You need to call
talloc_get_type_abort before the first use of ts.

> +    assert_non_null(data);

> +
> +    len = strlen(data) + 1;
> +    type = SSS_AUTHTOK_TYPE_PASSWORD;
> +    ts = talloc_get_type_abort(*state, struct test_state);
> +    ret = sss_authtok_set(ts->authtoken, type, (uint8_t *)data, len);
> +
> +    assert_int_equal(ret, EOK);
> +    assert_int_equal(type, sss_authtok_get_type(ts->authtoken));
> +    assert_int_equal(len, sss_authtok_get_size(ts->authtoken));
> +    assert_string_equal(data, sss_authtok_get_data(ts->authtoken));
> +
> +    ret = sss_authtok_get_password(ts->authtoken, &pwd, &ret_len);
> +
> +    assert_int_equal(ret, EOK);
> +    assert_string_equal(data, pwd);
> +    assert_int_equal(len - 1, ret_len);
> +
> +    ret = sss_authtok_set_password(ts->authtoken, data, len);
> +
> +    assert_int_equal(ret, EOK);
> +    ret = sss_authtok_get_password(ts->authtoken, &pwd, &ret_len);
> +    assert_int_equal(ret, EOK);
> +    assert_string_equal(data, pwd);
> +    assert_int_equal(len - 1, ret_len);
> +}
> +

> +/* Test when type has value SSS_AUTHTOK_TYPE_CCFILE */
> +static void test_sss_authtok_ccfile(void **state)
> +{
> +    size_t len;
> +    errno_t ret;
> +    const char *data;
> +    size_t ret_len;
> +    const char *pwd;
> +    struct test_state *ts;
> +    enum sss_authtok_type type;
> +

> +    data = talloc_strdup(ts, "path/to/cc_file");

Here ts is used uninitialized as well.

> +    assert_non_null(data);

> +
> +    len = strlen(data) + 1;
> +    type = SSS_AUTHTOK_TYPE_CCFILE;
> +    ts = talloc_get_type_abort(*state, struct test_state);
> +    ret = sss_authtok_set(ts->authtoken, type, (uint8_t *)data, len);
> +
> +    assert_int_equal(ret, EOK);
> +    assert_int_equal(type, sss_authtok_get_type(ts->authtoken));
> +    assert_int_equal(len, sss_authtok_get_size(ts->authtoken));
> +    assert_string_equal(data, sss_authtok_get_data(ts->authtoken));
> +
> +    ret = sss_authtok_get_ccfile(ts->authtoken, &pwd, &ret_len);
> +
> +    assert_int_equal(ret, EOK);
> +    assert_string_equal(data, pwd);
> +    assert_int_equal(len - 1, ret_len);
> +
> +    ret = sss_authtok_set_ccfile(ts->authtoken, data, len);

This line was giving me a compilation warning because data is const char
and set_ccfile expects uint8_t. You can simple cast data to uint8_t *

    ret = sss_authtok_set_ccfile(ts->authtoken, (uint8_t *) data, len);

> +
> +    assert_int_equal(ret, EOK);
> +
> +    ret = sss_authtok_get_ccfile(ts->authtoken, &pwd, &ret_len);
> +
> +    assert_int_equal(ret, EOK);
> +    assert_string_equal(data, pwd);
> +    assert_int_equal(len - 1, ret_len);
> +
> +

> +    ret = sss_authtok_set(ts->authtoken, type, data, 0);
> +
> +    assert_int_equal(ret, EOK);
> +    assert_int_equal(type, sss_authtok_get_type(ts->authtoken));
> +    assert_int_equal(len, sss_authtok_get_size(ts->authtoken));
> +    assert_string_equal(data, sss_authtok_get_data(ts->authtoken));
> +
> +    ret = sss_authtok_get_ccfile(ts->authtoken, &pwd, &ret_len);
> +
> +    assert_int_equal(ret, EOK);
> +    assert_string_equal(data, pwd);
> +    assert_int_equal(len - 1, ret_len);
> +}
> +
> +/* Test when type has value SSS_AUTHTOK_TYPE_EMPTY */
> +static void test_sss_authtok_empty(void **state)
> +{
> +    errno_t ret;
> +    size_t ret_len;
> +    const char *pwd;
> +    struct test_state *ts;
> +    enum sss_authtok_type type;
> +
> +    type = SSS_AUTHTOK_TYPE_EMPTY;
> +    ts = talloc_get_type_abort(*state, struct test_state);
> +    ret = sss_authtok_set(ts->authtoken, type, NULL, 0);
> +
> +    assert_int_equal(ret, EOK);
> +    assert_int_equal(type, sss_authtok_get_type(ts->authtoken));
> +    assert_int_equal(0, sss_authtok_get_size(ts->authtoken));
> +    assert_null(sss_authtok_get_data(ts->authtoken));
> +
> +    ret = sss_authtok_get_password(ts->authtoken, &pwd, &ret_len);
> +
> +    assert_int_equal(ret, ENOENT);
> +

> +    ret = sss_authtok_get_ccfile(ts->authtoken, &pwd, &ret_len);

> +
> +    assert_int_equal(ret, ENOENT);
> +

> +    sss_authtok_set_empty(ts->authtoken);
> +
> +    assert_int_equal(type, sss_authtok_get_type(ts->authtoken));
> +    assert_int_equal(0, sss_authtok_get_size(ts->authtoken));
> +    assert_null(sss_authtok_get_data(ts->authtoken));
> +
> +    ret = sss_authtok_set(ts->authtoken, type, '\0', 0);
> +    assert_int_equal(ret, EOK);
> +
> +    assert_int_equal(type, sss_authtok_get_type(ts->authtoken));

> +    assert_int_equal(EOK, sss_authtok_get_size(ts->authtoken));

> +    assert_null(sss_authtok_get_data(ts->authtoken));
> +
> +    ret = sss_authtok_get_password(ts->authtoken, &pwd, &ret_len);
> +
> +    assert_int_equal(ret, ENOENT);
> +

> +    ret = sss_authtok_get_ccfile(ts->authtoken, &pwd, &ret_len);

> +
> +    assert_int_equal(ret, ENOENT);
> +}
> +

> +static void test_sss_authtok_wipe_password(void **state)
> +{
> +    size_t len;
> +    errno_t ret;

> +    const char *data;
> +    size_t ret_len;
> +    const char *pwd;
> +    struct test_state *ts;
> +    enum sss_authtok_type type;

And also here.

> +
> +    data = talloc_strdup(ts, "password");
> +    assert_non_null(data);

> +
> +    len = strlen(data) + 1;
> +    type = SSS_AUTHTOK_TYPE_PASSWORD;
> +    ts = talloc_get_type_abort(*state, struct test_state);
> +    ret = sss_authtok_set(ts->authtoken, type, (uint8_t *)data, len);
> +
> +    assert_int_equal(ret, EOK);
> +

> +    sss_authtok_wipe_password(ts->authtoken);
> +
> +    ret = sss_authtok_get_password(ts->authtoken, &pwd, &ret_len);
> +
> +    assert_int_equal(ret, EOK);
> +    assert_string_equal(pwd, "");
> +    assert_int_equal(len - 1, ret_len);
> +}
> +