Skip to content

Missing error check of several functions #15524

@Hxinrong

Description

@Hxinrong

Hi,
According to the RIOT API, ds3231_get_time() and ds3231_set_time() are used to get or set date and time from the device, and return 0 on success, or -EIO on I2C communication error. But as shown below, the return values of them have not been checked.

ds3231_get_time(&_dev, &time);

ds3231_set_time(&_dev, &target_time);

And in the same file, at the following call sites, they are error checked,
res = ds3231_get_time(&_dev, &time);
if (res != 0) {
puts("error: unable to read time");
return 1;
}

res = ds3231_set_time(&_dev, &_riot_bday);
if (res != 0) {
puts("error: unable to set time");
return 1;
}


Function aes_encrypt() is to encrypt one plainBlock-block and save the result in cipherblock. It returns 1 on success, and a negative value if the cipher key cannot be expanded with the AES key schedule. In the following codes, there may be need an error check.

for (size_t i = 0; i < blocks; i++) {
aes_encrypt(&cipher, state->gen.counter.bytes, out + (i * 16));
fortuna_increment_counter(state);
}

However, there are error checks from other call sites, for example,
err = aes_encrypt(&ctx, TEST_0_INP, data);
TEST_ASSERT_EQUAL_INT(1, err);


The function cord_common_add_qstring() is used to add selected query string options to a gcoap request, and returns 0 on success, or a negative value on error. As follows, there could be add a check of the return value.

cord_common_add_qstring(&pkt);

What's more, we can see a return value check of it in the fragment below.
/* add Uri-Query options */
if (cord_common_add_qstring(&pkt) < 0) {
return CORD_EPSIM_ERROR;
}


Also, there may be a lack of error checks of function devfs_register(). It is to register a node in DevFS, and returns 0 on success, or a negative value on error. As shown below,

RIOT/boards/mulle/board.c

Lines 177 to 178 in c337089

/* Register DevFS node */
devfs_register(&mulle_nvram_devfs);

RIOT/boards/mulle/board.c

Lines 202 to 203 in c337089

/* Register DevFS node */
devfs_register(&mulle_nor_devfs);

Moreover, there is a function test_devfs_register() to test the return values of devfs_register() at the following file. So I think, there will be a need of error checks at the above call sites.

RIOT/tests/devfs/main.c

Lines 99 to 115 in c337089

static void test_devfs_register(void)
{
int res = devfs_register(NULL);
TEST_ASSERT(res < 0);
res = devfs_register(&_mock_devfs_node);
TEST_ASSERT(res == 0);
res = devfs_register(&_mock_devfs_node);
TEST_ASSERT(res < 0);
res = devfs_unregister(&_mock_devfs_node);
TEST_ASSERT(res == 0);
res = devfs_unregister(&_mock_devfs_node);
TEST_ASSERT(res < 0);
}

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type: enhancementThe issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions