-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Missing error check of several functions #15524
Description
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.
RIOT/tests/driver_ds3231/main.c
Line 97 in c337089
| ds3231_get_time(&_dev, &time); |
RIOT/tests/driver_ds3231/main.c
Line 125 in c337089
| ds3231_set_time(&_dev, &target_time); |
And in the same file, at the following call sites, they are error checked,
RIOT/tests/driver_ds3231/main.c
Lines 228 to 232 in c337089
| res = ds3231_get_time(&_dev, &time); | |
| if (res != 0) { | |
| puts("error: unable to read time"); | |
| return 1; | |
| } |
RIOT/tests/driver_ds3231/main.c
Lines 221 to 225 in c337089
| 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.
RIOT/sys/random/fortuna/fortuna.c
Lines 69 to 72 in c337089
| 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,
RIOT/tests/sys_crypto/tests-crypto-aes.c
Lines 52 to 53 in c337089
| 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.
RIOT/sys/net/application_layer/cord/epsim/cord_epsim.c
Lines 63 to 66 in c337089
| /* 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,
Lines 177 to 178 in c337089
| /* Register DevFS node */ | |
| devfs_register(&mulle_nvram_devfs); |
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.
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); | |
| } |