* [PATCH V3] nvme: code cleanup nvme_identify_ns_desc()
@ 2020-02-19 16:14 Chaitanya Kulkarni
2020-02-19 16:22 ` Christoph Hellwig
0 siblings, 1 reply; 3+ messages in thread
From: Chaitanya Kulkarni @ 2020-02-19 16:14 UTC (permalink / raw)
To: linux-nvme; +Cc: kbusch, hch, Chaitanya Kulkarni, sagi
The function nvme_identify_ns_desc() has 3 levels of nesting which make
error message to exceeded > 80 char per line which is not aligned with
the kernel code standards and rest of the NVMe subsystem code.
Add a helper function to move the processing of the log when the
command is successful by reducing the nesting and keeping the
code < 80 char per line.
Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
Changes from V2 (Christoph):-
1. Don't mix the coding styles for function arguments in the header.
2. Get rid of len variable in nvme_process_ns_desc().
3. Remove extra lines.
Changes from V1: -
1. Pass struct nvme_ns_id_desc *cur as it is to avoid data and pos
parameters to have a slightly cleaner patch. (Keith)
---
drivers/nvme/host/core.c | 76 +++++++++++++++++++++-------------------
1 file changed, 40 insertions(+), 36 deletions(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index cd0b47ed219e..84914223c537 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1055,6 +1055,43 @@ static int nvme_identify_ctrl(struct nvme_ctrl *dev, struct nvme_id_ctrl **id)
return error;
}
+static int nvme_process_ns_desc(struct nvme_ctrl *ctrl, struct nvme_ns_ids *ids,
+ struct nvme_ns_id_desc *cur)
+{
+ const char *warn_str = "ctrl returned bogus length:";
+ void *data = cur;
+
+ switch (cur->nidt) {
+ case NVME_NIDT_EUI64:
+ if (cur->nidl != NVME_NIDT_EUI64_LEN) {
+ dev_warn(ctrl->device, "%s %d for NVME_NIDT_EUI64\n",
+ warn_str, cur->nidl);
+ return -1;
+ }
+ memcpy(ids->eui64, data + sizeof(*cur), NVME_NIDT_EUI64_LEN);
+ return NVME_NIDT_EUI64_LEN;
+ case NVME_NIDT_NGUID:
+ if (cur->nidl != NVME_NIDT_NGUID_LEN) {
+ dev_warn(ctrl->device, "%s %d for NVME_NIDT_NGUID\n",
+ warn_str, cur->nidl);
+ return -1;
+ }
+ memcpy(ids->nguid, data + sizeof(*cur), NVME_NIDT_NGUID_LEN);
+ return NVME_NIDT_NGUID_LEN;
+ case NVME_NIDT_UUID:
+ if (cur->nidl != NVME_NIDT_UUID_LEN) {
+ dev_warn(ctrl->device, "%s %d for NVME_NIDT_UUID\n",
+ warn_str, cur->nidl);
+ return -1;
+ }
+ uuid_copy(&ids->uuid, data + sizeof(*cur));
+ return NVME_NIDT_UUID_LEN;
+ default:
+ /* Skip unknown types */
+ return cur->nidl;
+ }
+}
+
static int nvme_identify_ns_descs(struct nvme_ctrl *ctrl, unsigned nsid,
struct nvme_ns_ids *ids)
{
@@ -1083,42 +1120,9 @@ static int nvme_identify_ns_descs(struct nvme_ctrl *ctrl, unsigned nsid,
if (cur->nidl == 0)
break;
- switch (cur->nidt) {
- case NVME_NIDT_EUI64:
- if (cur->nidl != NVME_NIDT_EUI64_LEN) {
- dev_warn(ctrl->device,
- "ctrl returned bogus length: %d for NVME_NIDT_EUI64\n",
- cur->nidl);
- goto free_data;
- }
- len = NVME_NIDT_EUI64_LEN;
- memcpy(ids->eui64, data + pos + sizeof(*cur), len);
- break;
- case NVME_NIDT_NGUID:
- if (cur->nidl != NVME_NIDT_NGUID_LEN) {
- dev_warn(ctrl->device,
- "ctrl returned bogus length: %d for NVME_NIDT_NGUID\n",
- cur->nidl);
- goto free_data;
- }
- len = NVME_NIDT_NGUID_LEN;
- memcpy(ids->nguid, data + pos + sizeof(*cur), len);
- break;
- case NVME_NIDT_UUID:
- if (cur->nidl != NVME_NIDT_UUID_LEN) {
- dev_warn(ctrl->device,
- "ctrl returned bogus length: %d for NVME_NIDT_UUID\n",
- cur->nidl);
- goto free_data;
- }
- len = NVME_NIDT_UUID_LEN;
- uuid_copy(&ids->uuid, data + pos + sizeof(*cur));
- break;
- default:
- /* Skip unknown types */
- len = cur->nidl;
- break;
- }
+ len = nvme_process_ns_desc(ctrl, ids, cur);
+ if (len < 0)
+ goto free_data;
len += sizeof(*cur);
}
--
2.22.1
_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH V3] nvme: code cleanup nvme_identify_ns_desc()
2020-02-19 16:14 [PATCH V3] nvme: code cleanup nvme_identify_ns_desc() Chaitanya Kulkarni
@ 2020-02-19 16:22 ` Christoph Hellwig
2020-02-19 16:43 ` Keith Busch
0 siblings, 1 reply; 3+ messages in thread
From: Christoph Hellwig @ 2020-02-19 16:22 UTC (permalink / raw)
To: Chaitanya Kulkarni; +Cc: kbusch, hch, linux-nvme, sagi
On Wed, Feb 19, 2020 at 08:14:31AM -0800, Chaitanya Kulkarni wrote:
> The function nvme_identify_ns_desc() has 3 levels of nesting which make
> error message to exceeded > 80 char per line which is not aligned with
> the kernel code standards and rest of the NVMe subsystem code.
>
> Add a helper function to move the processing of the log when the
> command is successful by reducing the nesting and keeping the
> code < 80 char per line.
>
> Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH V3] nvme: code cleanup nvme_identify_ns_desc()
2020-02-19 16:22 ` Christoph Hellwig
@ 2020-02-19 16:43 ` Keith Busch
0 siblings, 0 replies; 3+ messages in thread
From: Keith Busch @ 2020-02-19 16:43 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-nvme, Chaitanya Kulkarni, sagi
Thanks, applied to nvme-5.7.
_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-02-19 16:43 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-19 16:14 [PATCH V3] nvme: code cleanup nvme_identify_ns_desc() Chaitanya Kulkarni
2020-02-19 16:22 ` Christoph Hellwig
2020-02-19 16:43 ` Keith Busch
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).