Skip to content

Simplify code flow in fabrics.c#3447

Open
Mr-Bossman wants to merge 2 commits into
linux-nvme:masterfrom
Mr-Bossman:dev/jesse/updates
Open

Simplify code flow in fabrics.c#3447
Mr-Bossman wants to merge 2 commits into
linux-nvme:masterfrom
Mr-Bossman:dev/jesse/updates

Conversation

@Mr-Bossman

Copy link
Copy Markdown
Contributor

hook_parser_next_line used gotos to implement a do-while loop.
Replace the gotos with a do-while loop to improve readability.
Simplify create_common_context by calling setup_common_context.
Add to set_fabrics_options to setup_common_context as both callers call it after setup_common_context.

Signed-off-by: Jesse Taube jtaubepe@redhat.com

Simplify create_common_context by calling setup_common_context.
Add to set_fabrics_options to setup_common_context as both callers
call it after setup_common_context.

Signed-off-by: Jesse Taube <jtaubepe@redhat.com>
hook_parser_next_line used gotos to implement a do-while loop.
Replace the gotos with a do-while loop to improve readability.

Signed-off-by: Jesse Taube <jtaubepe@redhat.com>
Comment thread fabrics.c

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's use then the opportunity to do return setup_comm_context(...) here.

Comment thread fabrics.c
@@ -391,7 +387,7 @@ static int setup_common_context(struct libnvmf_context *fctx,
if (err)
return err;

return 0;
return set_fabrics_options(fctx, fa);
}

static int create_common_context(struct libnvme_global_ctx *ctx,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose it would make sense to drop the common in the function names. I introduced it because I thought there will be a need to differentiate between the 'base' info and the rest. But looks like it's not really the case.

@igaw

igaw commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator

Looks good, just my nitpicks and a rebase is necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants