Remove any unnecessary curly braces and combine 'else' and 'if' to an 'else if' to improve the code's readability and reduce indentation.
Signed-off-by: Thorsten Blum thorsten.blum@linux.dev --- drivers/greybus/operation.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/greybus/operation.c b/drivers/greybus/operation.c index 8459e9bc0749..ba26504ccac3 100644 --- a/drivers/greybus/operation.c +++ b/drivers/greybus/operation.c @@ -1157,16 +1157,12 @@ int gb_operation_sync_timeout(struct gb_connection *connection, int type, memcpy(operation->request->payload, request, request_size);
ret = gb_operation_request_send_sync_timeout(operation, timeout); - if (ret) { + if (ret) dev_err(&connection->hd->dev, "%s: synchronous operation id 0x%04x of type 0x%02x failed: %d\n", connection->name, operation->id, type, ret); - } else { - if (response_size) { - memcpy(response, operation->response->payload, - response_size); - } - } + else if (response_size) + memcpy(response, operation->response->payload, response_size);
gb_operation_put(operation);
On Sun, Apr 13, 2025 at 12:48:03PM +0200, Thorsten Blum wrote:
Remove any unnecessary curly braces and combine 'else' and 'if' to an 'else if' to improve the code's readability and reduce indentation.
Signed-off-by: Thorsten Blum thorsten.blum@linux.dev
drivers/greybus/operation.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/greybus/operation.c b/drivers/greybus/operation.c index 8459e9bc0749..ba26504ccac3 100644 --- a/drivers/greybus/operation.c +++ b/drivers/greybus/operation.c @@ -1157,16 +1157,12 @@ int gb_operation_sync_timeout(struct gb_connection *connection, int type, memcpy(operation->request->payload, request, request_size); ret = gb_operation_request_send_sync_timeout(operation, timeout);
- if (ret) {
- if (ret) dev_err(&connection->hd->dev, "%s: synchronous operation id 0x%04x of type 0x%02x failed: %d\n", connection->name, operation->id, type, ret);
- } else {
if (response_size) {memcpy(response, operation->response->payload,response_size);}- }
- else if (response_size)
memcpy(response, operation->response->payload, response_size);
NAK
This just makes the code *less* readable.
Johan
On 14. Apr 2025, at 09:05, Johan Hovold wrote:
This just makes the code *less* readable.
I guess you prefer the code with curly braces?
What about the Linux kernel coding style [1]? Specifically "Do not unnecessarily use braces where a single statement will do."
My patch just removes any unnecessary curly braces, resulting in less lines of code and no line break in the memcpy() arguments.
Thanks, Thorsten
[1] https://docs.kernel.org/process/coding-style.html#placing-braces-and-spaces
On Mon, Apr 14, 2025 at 05:00:41PM +0200, Thorsten Blum wrote:
On 14. Apr 2025, at 09:05, Johan Hovold wrote:
This just makes the code *less* readable.
I guess you prefer the code with curly braces?
Around multiline statements yes, but the proposed if-else here also obscures the logic for no good reason.
What about the Linux kernel coding style [1]? Specifically "Do not unnecessarily use braces where a single statement will do."
My patch just removes any unnecessary curly braces, resulting in less lines of code and no line break in the memcpy() arguments.
I really don't care, the code is more readable as it stands which is what matters.
If you want to practise sending patches you can send all the "cleanups" you want for code in drivers/staging where churn like this may be accepted (and where the core greybus code no longer lives, contrary to what your Subject suggests).
Johan
-
Johan Hovold -
Thorsten Blum