Re: [PATCH v5 2/6] w1: ds2438: fixed if brackets coding style issue
On Fri, Apr 09, 2021 at 07:40:57AM -0700, Joe Perches wrote: > On Fri, 2021-04-09 at 00:09 -0300, Luiz Sampaio wrote: > > Since there is only one statement inside the if clause, no brackets are > > required. > [] > > diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c > [] > > @@ -287,9 +287,9 @@ static ssize_t iad_read(struct file *filp, struct > > kobject *kobj, > > if (!buf) > > return -EINVAL; > > > > > > - if (w1_ds2438_get_current(sl, ) == 0) { > > + if (w1_ds2438_get_current(sl, ) == 0) > > ret = snprintf(buf, count, "%i\n", voltage); > > - } else > > + else > > ret = -EIO; > > > > > > return ret; > > to me this would look better using a style like the below: > (and it might be better using sysfs_emit and not snprintf too) > > --- > drivers/w1/slaves/w1_ds2438.c | 36 > 1 file changed, 12 insertions(+), 24 deletions(-) > > diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c > index 5cfb0ae23e91..9115c5a9bc4f 100644 > --- a/drivers/w1/slaves/w1_ds2438.c > +++ b/drivers/w1/slaves/w1_ds2438.c > @@ -279,7 +279,6 @@ static ssize_t iad_read(struct file *filp, struct kobject > *kobj, > loff_t off, size_t count) > { > struct w1_slave *sl = kobj_to_w1_slave(kobj); > - int ret; > int16_t voltage; > > if (off != 0) > @@ -287,12 +286,10 @@ static ssize_t iad_read(struct file *filp, struct > kobject *kobj, > if (!buf) > return -EINVAL; > > - if (w1_ds2438_get_current(sl, ) == 0) { > - ret = snprintf(buf, count, "%i\n", voltage); > - } else > - ret = -EIO; > + if (w1_ds2438_get_current(sl, )) > + return -EIO; > > - return ret; > + return snprintf(buf, count, "%i\n", voltage); > } > > static ssize_t page0_read(struct file *filp, struct kobject *kobj, > @@ -330,7 +327,6 @@ static ssize_t temperature_read(struct file *filp, struct > kobject *kobj, > loff_t off, size_t count) > { > struct w1_slave *sl = kobj_to_w1_slave(kobj); > - int ret; > int16_t temp; > > if (off != 0) > @@ -338,12 +334,10 @@ static ssize_t temperature_read(struct file *filp, > struct kobject *kobj, > if (!buf) > return -EINVAL; > > - if (w1_ds2438_get_temperature(sl, ) == 0) { > - ret = snprintf(buf, count, "%i\n", temp); > - } else > - ret = -EIO; > + if (w1_ds2438_get_temperature(sl, )) > + return -EIO; > > - return ret; > + return snprintf(buf, count, "%i\n", temp); > } > > static ssize_t vad_read(struct file *filp, struct kobject *kobj, > @@ -351,7 +345,6 @@ static ssize_t vad_read(struct file *filp, struct kobject > *kobj, > loff_t off, size_t count) > { > struct w1_slave *sl = kobj_to_w1_slave(kobj); > - int ret; > uint16_t voltage; > > if (off != 0) > @@ -359,12 +352,10 @@ static ssize_t vad_read(struct file *filp, struct > kobject *kobj, > if (!buf) > return -EINVAL; > > - if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VAD, ) == 0) { > - ret = snprintf(buf, count, "%u\n", voltage); > - } else > - ret = -EIO; > + if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VAD, )) > + return -EIO; > > - return ret; > + return snprintf(buf, count, "%u\n", voltage); > } > > static ssize_t vdd_read(struct file *filp, struct kobject *kobj, > @@ -372,7 +363,6 @@ static ssize_t vdd_read(struct file *filp, struct kobject > *kobj, > loff_t off, size_t count) > { > struct w1_slave *sl = kobj_to_w1_slave(kobj); > - int ret; > uint16_t voltage; > > if (off != 0) > @@ -380,12 +370,10 @@ static ssize_t vdd_read(struct file *filp, struct > kobject *kobj, > if (!buf) > return -EINVAL; > > - if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VDD, ) == 0) { > - ret = snprintf(buf, count, "%u\n", voltage); > - } else > - ret = -EIO; > + if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VDD, )) > + return -EIO; > > - return ret; > + return snprintf(buf, count, "%u\n", voltage); > } > > static BIN_ATTR(iad, S_IRUGO | S_IWUSR | S_IWGRP, iad_read, iad_write, 0); > Sorry for the late reply! I agree, this would look nicer. I will wait for the current revision and change this for the next one.
Re: [PATCH v5 2/6] w1: ds2438: fixed if brackets coding style issue
On Fri, 2021-04-09 at 00:09 -0300, Luiz Sampaio wrote: > Since there is only one statement inside the if clause, no brackets are > required. [] > diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c [] > @@ -287,9 +287,9 @@ static ssize_t iad_read(struct file *filp, struct kobject > *kobj, > if (!buf) > return -EINVAL; > > > - if (w1_ds2438_get_current(sl, ) == 0) { > + if (w1_ds2438_get_current(sl, ) == 0) > ret = snprintf(buf, count, "%i\n", voltage); > - } else > + else > ret = -EIO; > > > return ret; to me this would look better using a style like the below: (and it might be better using sysfs_emit and not snprintf too) --- drivers/w1/slaves/w1_ds2438.c | 36 1 file changed, 12 insertions(+), 24 deletions(-) diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c index 5cfb0ae23e91..9115c5a9bc4f 100644 --- a/drivers/w1/slaves/w1_ds2438.c +++ b/drivers/w1/slaves/w1_ds2438.c @@ -279,7 +279,6 @@ static ssize_t iad_read(struct file *filp, struct kobject *kobj, loff_t off, size_t count) { struct w1_slave *sl = kobj_to_w1_slave(kobj); - int ret; int16_t voltage; if (off != 0) @@ -287,12 +286,10 @@ static ssize_t iad_read(struct file *filp, struct kobject *kobj, if (!buf) return -EINVAL; - if (w1_ds2438_get_current(sl, ) == 0) { - ret = snprintf(buf, count, "%i\n", voltage); - } else - ret = -EIO; + if (w1_ds2438_get_current(sl, )) + return -EIO; - return ret; + return snprintf(buf, count, "%i\n", voltage); } static ssize_t page0_read(struct file *filp, struct kobject *kobj, @@ -330,7 +327,6 @@ static ssize_t temperature_read(struct file *filp, struct kobject *kobj, loff_t off, size_t count) { struct w1_slave *sl = kobj_to_w1_slave(kobj); - int ret; int16_t temp; if (off != 0) @@ -338,12 +334,10 @@ static ssize_t temperature_read(struct file *filp, struct kobject *kobj, if (!buf) return -EINVAL; - if (w1_ds2438_get_temperature(sl, ) == 0) { - ret = snprintf(buf, count, "%i\n", temp); - } else - ret = -EIO; + if (w1_ds2438_get_temperature(sl, )) + return -EIO; - return ret; + return snprintf(buf, count, "%i\n", temp); } static ssize_t vad_read(struct file *filp, struct kobject *kobj, @@ -351,7 +345,6 @@ static ssize_t vad_read(struct file *filp, struct kobject *kobj, loff_t off, size_t count) { struct w1_slave *sl = kobj_to_w1_slave(kobj); - int ret; uint16_t voltage; if (off != 0) @@ -359,12 +352,10 @@ static ssize_t vad_read(struct file *filp, struct kobject *kobj, if (!buf) return -EINVAL; - if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VAD, ) == 0) { - ret = snprintf(buf, count, "%u\n", voltage); - } else - ret = -EIO; + if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VAD, )) + return -EIO; - return ret; + return snprintf(buf, count, "%u\n", voltage); } static ssize_t vdd_read(struct file *filp, struct kobject *kobj, @@ -372,7 +363,6 @@ static ssize_t vdd_read(struct file *filp, struct kobject *kobj, loff_t off, size_t count) { struct w1_slave *sl = kobj_to_w1_slave(kobj); - int ret; uint16_t voltage; if (off != 0) @@ -380,12 +370,10 @@ static ssize_t vdd_read(struct file *filp, struct kobject *kobj, if (!buf) return -EINVAL; - if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VDD, ) == 0) { - ret = snprintf(buf, count, "%u\n", voltage); - } else - ret = -EIO; + if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VDD, )) + return -EIO; - return ret; + return snprintf(buf, count, "%u\n", voltage); } static BIN_ATTR(iad, S_IRUGO | S_IWUSR | S_IWGRP, iad_read, iad_write, 0);
Re: [PATCH v5 2/6] w1: ds2438: fixed if brackets coding style issue
On Fri, 2021-04-09 at 00:09 -0300, Luiz Sampaio wrote: > Since there is only one statement inside the if clause, no brackets are > required. > > Signed-off-by: Luiz Sampaio > --- > drivers/w1/slaves/w1_ds2438.c | 16 > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c > index 148921fb9702..56e53a748059 100644 > --- a/drivers/w1/slaves/w1_ds2438.c > +++ b/drivers/w1/slaves/w1_ds2438.c > @@ -287,9 +287,9 @@ static ssize_t iad_read(struct file *filp, struct kobject > *kobj, > if (!buf) > return -EINVAL; > > > - if (w1_ds2438_get_current(sl, ) == 0) { > + if (w1_ds2438_get_current(sl, ) == 0) > ret = snprintf(buf, count, "%i\n", voltage); > - } else > + else > ret = -EIO; > > > return ret; > @@ -338,9 +338,9 @@ static ssize_t temperature_read(struct file *filp, struct > kobject *kobj, > if (!buf) > return -EINVAL; > > > - if (w1_ds2438_get_temperature(sl, ) == 0) { > + if (w1_ds2438_get_temperature(sl, ) == 0) > ret = snprintf(buf, count, "%i\n", temp); > - } else > + else > ret = -EIO; > > > return ret; > @@ -359,9 +359,9 @@ static ssize_t vad_read(struct file *filp, struct kobject > *kobj, > if (!buf) > return -EINVAL; > > > - if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VAD, ) == 0) { > + if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VAD, ) == 0) > ret = snprintf(buf, count, "%u\n", voltage); > - } else > + else > ret = -EIO; > > > return ret; > @@ -380,9 +380,9 @@ static ssize_t vdd_read(struct file *filp, struct kobject > *kobj, > if (!buf) > return -EINVAL; > > > - if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VDD, ) == 0) { > + if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VDD, ) == 0) > ret = snprintf(buf, count, "%u\n", voltage); > - } else > + else > ret = -EIO; > > > return ret;
[PATCH v5 2/6] w1: ds2438: fixed if brackets coding style issue
Since there is only one statement inside the if clause, no brackets are required. Signed-off-by: Luiz Sampaio --- drivers/w1/slaves/w1_ds2438.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c index 148921fb9702..56e53a748059 100644 --- a/drivers/w1/slaves/w1_ds2438.c +++ b/drivers/w1/slaves/w1_ds2438.c @@ -287,9 +287,9 @@ static ssize_t iad_read(struct file *filp, struct kobject *kobj, if (!buf) return -EINVAL; - if (w1_ds2438_get_current(sl, ) == 0) { + if (w1_ds2438_get_current(sl, ) == 0) ret = snprintf(buf, count, "%i\n", voltage); - } else + else ret = -EIO; return ret; @@ -338,9 +338,9 @@ static ssize_t temperature_read(struct file *filp, struct kobject *kobj, if (!buf) return -EINVAL; - if (w1_ds2438_get_temperature(sl, ) == 0) { + if (w1_ds2438_get_temperature(sl, ) == 0) ret = snprintf(buf, count, "%i\n", temp); - } else + else ret = -EIO; return ret; @@ -359,9 +359,9 @@ static ssize_t vad_read(struct file *filp, struct kobject *kobj, if (!buf) return -EINVAL; - if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VAD, ) == 0) { + if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VAD, ) == 0) ret = snprintf(buf, count, "%u\n", voltage); - } else + else ret = -EIO; return ret; @@ -380,9 +380,9 @@ static ssize_t vdd_read(struct file *filp, struct kobject *kobj, if (!buf) return -EINVAL; - if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VDD, ) == 0) { + if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VDD, ) == 0) ret = snprintf(buf, count, "%u\n", voltage); - } else + else ret = -EIO; return ret; -- 2.30.1