Discussion:
[PATCH 0/2] staging: greybus: Fix parameters alignment and strings concatenation
Cristian Sicilia
2018-12-04 20:58:07 UTC
Permalink
First patch align some parameters with parenthesis.
Second patch will add some spaces between string.

Cristian Sicilia (2):
staging: greybus: Align function call parameters to parenthesis
staging: greybus: Added space between string concatenated

drivers/staging/greybus/loopback.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
--
2.7.4
Cristian Sicilia
2018-12-04 20:58:14 UTC
Permalink
Aligned some parameters to the previous parenthesis.

Signed-off-by: Cristian Sicilia <***@gmail.com>
---
drivers/staging/greybus/loopback.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c
index e4d42c1..1085e06 100644
--- a/drivers/staging/greybus/loopback.c
+++ b/drivers/staging/greybus/loopback.c
@@ -638,7 +638,8 @@ static int gb_loopback_async_transfer(struct gb_loopback *gb, u32 len)
retval = gb_loopback_async_operation(gb, GB_LOOPBACK_TYPE_TRANSFER,
request, len + sizeof(*request),
len + response_len,
- gb_loopback_async_transfer_complete);
+ gb_loopback_async_transfer_complete
+ );
if (retval)
goto gb_error;

@@ -682,7 +683,8 @@ static int gb_loopback_request_handler(struct gb_operation *operation)
}

if (!gb_operation_response_alloc(operation,
- len + sizeof(*response), GFP_KERNEL)) {
+ len + sizeof(*response),
+ GFP_KERNEL)) {
dev_err(dev, "error allocating response\n");
return -ENOMEM;
}
@@ -882,7 +884,7 @@ static int gb_loopback_fn(void *data)
gb->type = 0;
gb->send_count = 0;
sysfs_notify(&gb->dev->kobj, NULL,
- "iteration_count");
+ "iteration_count");
dev_dbg(&bundle->dev, "load test complete\n");
} else {
dev_dbg(&bundle->dev,
@@ -1066,7 +1068,7 @@ static int gb_loopback_probe(struct gb_bundle *bundle,

/* Allocate kfifo */
if (kfifo_alloc(&gb->kfifo_lat, kfifo_depth * sizeof(u32),
- GFP_KERNEL)) {
+ GFP_KERNEL)) {
retval = -ENOMEM;
goto out_conn;
}
--
2.7.4
Greg Kroah-Hartman
2018-12-05 08:46:03 UTC
Permalink
Post by Cristian Sicilia
Aligned some parameters to the previous parenthesis.
---
drivers/staging/greybus/loopback.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c
index e4d42c1..1085e06 100644
--- a/drivers/staging/greybus/loopback.c
+++ b/drivers/staging/greybus/loopback.c
@@ -638,7 +638,8 @@ static int gb_loopback_async_transfer(struct gb_loopback *gb, u32 len)
retval = gb_loopback_async_operation(gb, GB_LOOPBACK_TYPE_TRANSFER,
request, len + sizeof(*request),
len + response_len,
- gb_loopback_async_transfer_complete);
+ gb_loopback_async_transfer_complete
+ );
Ick, why do this? That's not really needed.

thanks,

greg k-h
Sicilia Cristian
2018-12-05 21:09:51 UTC
Permalink
Post by Greg Kroah-Hartman
Post by Cristian Sicilia
Aligned some parameters to the previous parenthesis.
---
drivers/staging/greybus/loopback.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c
index e4d42c1..1085e06 100644
--- a/drivers/staging/greybus/loopback.c
+++ b/drivers/staging/greybus/loopback.c
@@ -638,7 +638,8 @@ static int gb_loopback_async_transfer(struct gb_loopback *gb, u32 len)
retval = gb_loopback_async_operation(gb, GB_LOOPBACK_TYPE_TRANSFER,
request, len + sizeof(*request),
len + response_len,
- gb_loopback_async_transfer_complete);
+ gb_loopback_async_transfer_complete
+ );
Ick, why do this? That's not really needed.
Ok, better to leave it unchanged
Post by Greg Kroah-Hartman
thanks,
greg k-h
Cristian Sicilia
2018-12-04 20:58:20 UTC
Permalink
Some concatenated strings are now spaced.

Signed-off-by: Cristian Sicilia <***@gmail.com>
---
drivers/staging/greybus/loopback.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c
index 1085e06..acfa392 100644
--- a/drivers/staging/greybus/loopback.c
+++ b/drivers/staging/greybus/loopback.c
@@ -141,7 +141,7 @@ static ssize_t name##_##field##_show(struct device *dev, \
/* Report 0 for min and max if no transfer successed */ \
if (!gb->requests_completed) \
return sprintf(buf, "0\n"); \
- return sprintf(buf, "%"#type"\n", gb->name.field); \
+ return sprintf(buf, "%" #type "\n", gb->name.field); \
} \
static DEVICE_ATTR_RO(name##_##field)

@@ -176,7 +176,7 @@ static ssize_t field##_show(struct device *dev, \
char *buf) \
{ \
struct gb_loopback *gb = dev_get_drvdata(dev); \
- return sprintf(buf, "%"#type"\n", gb->field); \
+ return sprintf(buf, "%" #type "\n", gb->field); \
} \
static ssize_t field##_store(struct device *dev, \
struct device_attribute *attr, \
@@ -212,7 +212,7 @@ static ssize_t field##_show(struct device *dev, \
char *buf) \
{ \
struct gb_loopback *gb = dev_get_drvdata(dev); \
- return sprintf(buf, "%"#type"\n", gb->field); \
+ return sprintf(buf, "%" #type "\n", gb->field); \
} \
static ssize_t field##_store(struct device *dev, \
struct device_attribute *attr, \
--
2.7.4
Bryan O'Donoghue
2018-12-05 10:12:24 UTC
Permalink
Post by Cristian Sicilia
Some concatenated strings are now spaced.
---
drivers/staging/greybus/loopback.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c
index 1085e06..acfa392 100644
--- a/drivers/staging/greybus/loopback.c
+++ b/drivers/staging/greybus/loopback.c
@@ -141,7 +141,7 @@ static ssize_t name##_##field##_show(struct device *dev, \
/* Report 0 for min and max if no transfer successed */ \
if (!gb->requests_completed) \
return sprintf(buf, "0\n"); \
- return sprintf(buf, "%"#type"\n", gb->name.field); \
+ return sprintf(buf, "%" #type "\n", gb->name.field); \
} \
static DEVICE_ATTR_RO(name##_##field)
@@ -176,7 +176,7 @@ static ssize_t field##_show(struct device *dev, \
char *buf) \
{ \
struct gb_loopback *gb = dev_get_drvdata(dev); \
- return sprintf(buf, "%"#type"\n", gb->field); \
+ return sprintf(buf, "%" #type "\n", gb->field); \
} \
static ssize_t field##_store(struct device *dev, \
struct device_attribute *attr, \
@@ -212,7 +212,7 @@ static ssize_t field##_show(struct device *dev, \
char *buf) \
{ \
struct gb_loopback *gb = dev_get_drvdata(dev); \
- return sprintf(buf, "%"#type"\n", gb->field); \
+ return sprintf(buf, "%" #type "\n", gb->field); \
} \
static ssize_t field##_store(struct device *dev, \
struct device_attribute *attr, \
Eh.

But doesn't all of this add an extra two space to the resultant string ?

Not what we want.

---
bod
Sicilia Cristian
2018-12-05 21:00:34 UTC
Permalink
Post by Bryan O'Donoghue
Post by Cristian Sicilia
Some concatenated strings are now spaced.
---
drivers/staging/greybus/loopback.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c
index 1085e06..acfa392 100644
--- a/drivers/staging/greybus/loopback.c
+++ b/drivers/staging/greybus/loopback.c
@@ -141,7 +141,7 @@ static ssize_t name##_##field##_show(struct device *dev, \
/* Report 0 for min and max if no transfer successed */ \
if (!gb->requests_completed) \
return sprintf(buf, "0\n"); \
- return sprintf(buf, "%"#type"\n", gb->name.field); \
+ return sprintf(buf, "%" #type "\n", gb->name.field); \
} \
static DEVICE_ATTR_RO(name##_##field)
@@ -176,7 +176,7 @@ static ssize_t field##_show(struct device *dev, \
char *buf) \
{ \
struct gb_loopback *gb = dev_get_drvdata(dev); \
- return sprintf(buf, "%"#type"\n", gb->field); \
+ return sprintf(buf, "%" #type "\n", gb->field); \
} \
static ssize_t field##_store(struct device *dev, \
struct device_attribute *attr, \
@@ -212,7 +212,7 @@ static ssize_t field##_show(struct device *dev, \
char *buf) \
{ \
struct gb_loopback *gb = dev_get_drvdata(dev); \
- return sprintf(buf, "%"#type"\n", gb->field); \
+ return sprintf(buf, "%" #type "\n", gb->field); \
} \
static ssize_t field##_store(struct device *dev, \
struct device_attribute *attr, \
Eh.
But doesn't all of this add an extra two space to the resultant string ?
Not what we want.
It doesn't change the result string, if I well understand the question your
doubt is if there are some space between % and type or between type and
end of line:

This (supposing type=u and field=min):
return sprintf(buf, "%"#type"\n", gb->field);
Will expanded in this
return sprintf(buf, "%""u""\n", gb->min);
That's is like:
return sprintf(buf, "%u\n", gb->min);

This (supposing type=u and field=min):
return sprintf(buf, "%" #type "\n", gb->field);
Will expanded in this
return sprintf(buf, "%" "u" "\n", gb->min);
That's is like:
return sprintf(buf, "%u\n", gb->min);
Post by Bryan O'Donoghue
---
bod
Bryan O'Donoghue
2018-12-06 09:43:46 UTC
Permalink
Post by Sicilia Cristian
It doesn't change the result string
So why do it then ?
Greg Kroah-Hartman
2018-12-06 15:05:16 UTC
Permalink
Post by Bryan O'Donoghue
Post by Sicilia Cristian
It doesn't change the result string
So why do it then ?
Because it is easier to read this way and odds are checkpatch is
happier.

thanks,

greg k-h
Bryan O'Donoghue
2018-12-06 16:14:53 UTC
Permalink
Post by Greg Kroah-Hartman
Post by Bryan O'Donoghue
Post by Sicilia Cristian
It doesn't change the result string
So why do it then ?
Because it is easier to read this way and odds are checkpatch is
happier.
Fine.

Sicilia could you please amend your commit.

'This patch fixes the checkpatch error "CHECK: Concatenated strings
should use spaces between elements"'

and then feel free to add my

Acked-by: Bryan O'Donoghue <***@nexus-software.ie>

---
bod
Greg Kroah-Hartman
2018-12-06 16:23:40 UTC
Permalink
Post by Bryan O'Donoghue
Post by Greg Kroah-Hartman
Post by Bryan O'Donoghue
Post by Sicilia Cristian
It doesn't change the result string
So why do it then ?
Because it is easier to read this way and odds are checkpatch is
happier.
Fine.
Sicilia could you please amend your commit.
'This patch fixes the checkpatch error "CHECK: Concatenated strings should
use spaces between elements"'
and then feel free to add my
It's already in my tree, no need to ammend it, sorry :)

greg k-h
Bryan O'Donoghue
2018-12-06 16:24:21 UTC
Permalink
Post by Greg Kroah-Hartman
Post by Bryan O'Donoghue
Post by Greg Kroah-Hartman
Post by Bryan O'Donoghue
Post by Sicilia Cristian
It doesn't change the result string
So why do it then ?
Because it is easier to read this way and odds are checkpatch is
happier.
Fine.
Sicilia could you please amend your commit.
'This patch fixes the checkpatch error "CHECK: Concatenated strings should
use spaces between elements"'
and then feel free to add my
It's already in my tree, no need to ammend it, sorry :)
greg k-h
thanks greg !

Loading...