make: fix unused-params error by smlng · Pull Request #7995 · RIOT-OS/RIOT

smlng · 2017-11-10T09:02:22Z

This RP is required to achieve #7919, it fixes several unused-params errors.

cladmi

Changes look good, murdock still builds correctly, ACK.

smlng · 2017-11-10T10:45:04Z

@cladmi if you haven't already: please do not merge

smlng · 2017-11-10T10:45:56Z

according to #7919 there are lots more of these to fix, I'll try to append

Dismissed because of changes after ACK

miri64 · 2017-11-12T11:08:36Z

cpu/atmega_common/periph/i2c.c

-    assert(dev < I2C_NUMOF);
-    mutex_lock(&lock);
-    return 0;
+    if (dev < I2C_NUMOF) {


Here and below: I think there is a reason why the author chose assert() over if (). Better use (void) dev instead.

There seem to be the general consent that if () should be used instead of assert(), at least most platforms do, accept this and the nrf51. Maybe someone more into periph can give there opinion here, @gebart or @haukepetersen?

But if the change-over to if () should happen I would rather like to see it in a separate PR and also for nrf51 as well.

the reason for that is, that the API requires to return an error for invalid device, so an assert is just not enough here

shall I also factor this one out into a separate PR?

I'd prefer that.

miri64 · 2017-11-12T11:10:07Z

tests/driver_dynamixel/main.c

 #ifdef DXL_DIR_PIN
-static void dir_init(uart_t uart) {
+static void dir_init(uart_t uart)
+{


Here and below: unrelated style fixes, but okay.

miri64 · 2017-11-12T11:12:10Z

cpu/sam3/periph/dac.c

-    PMC->PMC_PCER1 = (1 << (ID_DACC - 32));
-    DACC->DACC_CHER = PMC_BIT;
+    DACC->DACC_CHER = (1 << line);
+    PMC->PMC_PCER1 = PMC_BIT;


Mhhhh this changes the functionality. From the surrounding lines I suspect this is correct, but I'm not sure. @haukepetersen can you confirm, please?

It would make the review easier if this was a separate PR.

okay I'll factor out.

smlng · 2017-11-13T10:20:22Z

~~requires #8013~~

smlng · 2017-11-13T12:29:46Z

~~requires #8017~~

cladmi

For all cases where a parameter is only used when assert is enabled, you can enclose them with #ifdef NDEBUG.
I tried listing them all (sorry for the repeated boring copy-pasted message).

Either than that, it looks good and trust murdock.

cladmi · 2017-11-14T15:37:25Z

cpu/atmega_common/periph/i2c.c

 void i2c_poweron(i2c_t dev)
 {
    assert(dev < I2C_NUMOF);
+    (void) dev;


You could enclose the (void) dev; with #ifdef NDEBUG

IMHO this will decrease readability and the compiler with optimise out one or the other anyway.

I agree with @smlng. This would lead to very ugly code.

cladmi · 2017-11-14T15:37:34Z

cpu/atmega_common/periph/i2c.c

 void i2c_poweroff(i2c_t dev)
 {
    assert(dev < I2C_NUMOF);
+    (void) dev;


You could enclose the (void) dev; with #ifdef NDEBUG

cladmi · 2017-11-14T15:39:21Z

cpu/mips_pic32mx/eic_pic32mx.c


 void eic_irq_configure(int irq_num)
 {
+    (void)irq_num;


You could enclose the (void) irq_num with #ifdef NDEBUG

cladmi · 2017-11-14T15:39:29Z

cpu/mips_pic32mx/eic_pic32mx.c


 void eic_irq_enable(int irq_num)
 {
+    (void)irq_num;


You could enclose the (void) irq_num with #ifdef NDEBUG

cladmi · 2017-11-14T15:39:35Z

cpu/mips_pic32mx/eic_pic32mx.c


 void eic_irq_disable(int irq_num)
 {
+    (void)irq_num;


You could enclose the (void) irq_num with #ifdef NDEBUG

cladmi · 2017-11-14T15:41:48Z

cpu/sam3/periph/pwm.c


 uint8_t pwm_channels(pwm_t pwm)
 {
+    (void)pwm;


You could enclose the (void) pwm with #ifdef NDEBUG

cladmi · 2017-11-14T15:41:54Z

cpu/sam3/periph/pwm.c

 */
 void pwm_set(pwm_t pwm, uint8_t channel, uint16_t value)
 {
+    (void)pwm;


You could enclose the (void) pwm with #ifdef NDEBUG

cladmi · 2017-11-14T15:41:59Z

cpu/sam3/periph/pwm.c


 void pwm_poweron(pwm_t pwm)
 {
+    (void)pwm;


You could enclose the (void) pwm with #ifdef NDEBUG

cladmi · 2017-11-14T15:42:04Z

cpu/sam3/periph/pwm.c


 void pwm_poweroff(pwm_t pwm)
 {
+    (void)pwm;


You could enclose the (void) pwm with #ifdef NDEBUG

cladmi · 2017-11-14T15:43:00Z

drivers/netdev_ieee802154/netdev_ieee802154.c


 static int _get_iid(netdev_ieee802154_t *dev, eui64_t *value, size_t max_len)
 {
+    (void)max_len;


You could enclose the (void)max_len with #ifdef NDEBUG

Choosen that no need to enclose the (void)var with #ifdef NDEBUG when only used by assert.

smlng · 2017-11-28T09:58:11Z

rebased, may I squash?

miri64

ACK

miri64 · 2017-11-28T11:13:53Z

(yes, please squash)

cpu, sam0_common: fix unused parameter in periph/spi cpu, kinetis_common: fix unused parameter in periph/spi cpu, cc2538: fix unused param in periph/i2c cpu, cc2538: fix unused param in periph/spi cpu, sam3: fix unused param in periph/spi cpu, stm32_common: fix unused param in periph/pm cpu, stm32f3: fix unused params in periph/i2c cpu, nrf5x_common: fix unused param in periph/gpio cpu, nrf5x_common: fix unused param in periph/spi cpu, lpc2387: fix unused params in periph/spi cpu, cc2538: fix unused params in radio/netdev cpu, cc2650: fix unused params in periph/uart cpu, lm4f120: fix unused param in periph/spi cpu, lm4f120: fix unused params in periph/timer cpu, lm4f120: fix unused params in periph/uart cpu, stm32_common: fix unused params in periph/dac cpu, stm32l0: fix unused params in periph/i2c cpu, msp430fxyz: fix unused params in periph/uart cpu, mips: fix unused params cpu, cc430: fix unused-params in periph/timer cpu, msp430fxyz: fix unused params in periph/spi drivers, cc2420: fix unused param cpu, mips32r2_common: fix unused params in periph/timer cpu, cc2538: fix unused-param in periph/i2c cpu, mips32r2_common: fix unused-param in periph/timer cpu, msp430fxyz: fix unused params in periph/timer cpu, atmega_common: fix unused params in periph/spi driver, nrfmin: fix unused params cpu, cc2538_rf: fix unused params driver, netdev_ieee802514: fix unused param cpu, mip_pic32m: fix unused params cpu, lpc2387: fix unused params in periph/pwm tests/driver_sdcard_spi: fix unused params cpu, sam3: fix unused param in periph/pwm tests/driver_dynamixel: fix unused params, and style issues cpu, cc430: fix unused param in periph/rtc cpu, atmega_common: fix unused params in periph/i2c

smlng · 2017-11-28T13:44:04Z

squashed

smlng added Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation Area: build system Area: Build system labels Nov 10, 2017

smlng mentioned this pull request Nov 10, 2017

Make: enable -Werror -Wall -Wextra by default #7919

Merged

smlng added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Nov 10, 2017

cladmi previously approved these changes Nov 10, 2017

View reviewed changes

smlng added the State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet label Nov 10, 2017

smlng changed the title ~~make: fix unused-params error~~ [WIP] make: fix unused-params error Nov 10, 2017

smlng force-pushed the make/fix/unused_params branch from 0e29aa8 to ee2f085 Compare November 10, 2017 15:24

smlng mentioned this pull request Nov 11, 2017

make: fix sign-compare errors #7994

Merged

smlng changed the title ~~[WIP] make: fix unused-params error~~ make: fix unused-params error Nov 11, 2017

smlng removed the State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet label Nov 11, 2017

miri64 requested changes Nov 12, 2017

View reviewed changes

miri64 assigned jnohlgard and miri64 Nov 12, 2017

smlng mentioned this pull request Nov 13, 2017

cpu, sam3: fix periph/dac poweron #8013

Merged

smlng force-pushed the make/fix/unused_params branch from 0d7d4a5 to 863352e Compare November 13, 2017 10:20

smlng added the State: waiting for other PR State: The PR requires another PR to be merged first label Nov 13, 2017

kaspar030 added the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Nov 13, 2017

smlng force-pushed the make/fix/unused_params branch 2 times, most recently from f791688 to 6af87dc Compare November 13, 2017 12:27

smlng mentioned this pull request Nov 13, 2017

cpu, atmega_common: fix returns and error codes in periph/i2c #8017

Merged

smlng mentioned this pull request Nov 13, 2017

nrf51: fix returns and error codes in periph/i2c #8018

Merged

cladmi previously requested changes Nov 14, 2017

View reviewed changes

cladmi approved these changes Nov 14, 2017

View reviewed changes

smlng force-pushed the make/fix/unused_params branch from 6af87dc to 3bc3b96 Compare November 28, 2017 09:42

smlng removed the State: waiting for other PR State: The PR requires another PR to be merged first label Nov 28, 2017

smlng added the Community: Hack'n'ACK candidate This PR is a candidate for review and discussion during one of RIOT's monthly Hack'n'ACK parties label Nov 28, 2017

miri64 approved these changes Nov 28, 2017

View reviewed changes

smlng force-pushed the make/fix/unused_params branch from 3bc3b96 to 7309171 Compare November 28, 2017 13:38

miri64 merged commit 39c1221 into RIOT-OS:master Nov 28, 2017

smlng deleted the make/fix/unused_params branch November 28, 2017 13:48

aabadie added this to the Release 2018.01 milestone Jan 18, 2018

Conversation

smlng commented Nov 10, 2017

Uh oh!

cladmi left a comment

Choose a reason for hiding this comment

Uh oh!

smlng commented Nov 10, 2017

Uh oh!

smlng commented Nov 10, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smlng commented Nov 13, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

smlng commented Nov 13, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cladmi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smlng commented Nov 28, 2017

Uh oh!

miri64 left a comment

Choose a reason for hiding this comment

Uh oh!

miri64 commented Nov 28, 2017

Uh oh!

smlng commented Nov 28, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

smlng commented Nov 13, 2017 •

edited

Loading

smlng commented Nov 13, 2017 •

edited

Loading