From: Julia Lawall <julia.lawall@inria.fr>
To: Dorcas Litunya <anonolitunya@gmail.com>
Cc: outreachy@lists.linux.dev
Subject: Re: [Outreachy]Clarification on code section in qlge driver
Date: Tue, 24 Oct 2023 14:41:22 +0200 (CEST) [thread overview]
Message-ID: <4642349f-c683-e5b-7be8-375490efd6@inria.fr> (raw)
In-Reply-To: <ZSjpGx+s6xCVOnKF@dorcaslitunya-virtual-machine>
On Fri, 13 Oct 2023, Dorcas Litunya wrote:
> On Fri, Oct 13, 2023 at 08:41:32AM +0200, Julia Lawall wrote:
> >
> >
> > On Fri, 13 Oct 2023, Dorcas Litunya wrote:
> >
> > > Hi Julia,
> > >
> > > I am trying to edit the above macro to incorporate an inline function as
> > > macros with control flows are highly discouraged. However, I wanted to
> > > inquire about some parameters in the FILL_SEG macro below so that I can be able effectively
> > > create a well parameterized function.
> >
> > seg_hdr and seg_regs are not vealues, they are field names. There is no
> > way that you can pass a field name to a function. You could pass the
> > complete values &dump->seg_hdr and dump->seg_regs, but then one could
> > wonder why not pass dump as well. And then the calls would start getting
> > a bit large.
> >
> > Since this macro is specific to a particular function and is defined right
> > before that function, I think it would be better to leave it as it is.
> >
> Makes a lot of sense. My other idea besides defining the entire macro as
> a function was to encapsulate what the macro does in the code block as a
> function as below:
>
> #define FILL_SEG(seg_hdr, seg_regs,fmsg,dump) fill_seg_handling_error(fmsg,dump)
>
> static int fill_seg_handling_error(struct fmsg *fmsg, struct dump *dump) {
> int err = qlge_fill_seg_(fmsg, &dump->seg_hdr, dump->seg_regs);
> if (err) {
> kvfree(dump);
> return err;
> }
> return 0
> }
>
> Not sure if doing it this way creates some sort of redudancy or may lead to some
> unprecedented problems or can it be considered
> as a way to make the code more readable and maintainable?
Sorry to hqave missed this mail. I think that the goal of the original
macro was to jump directly out of the function on failure, not to
have the code cluttered with tests, so this change would not give the full
benefit.
julia
>
> Dorcas
> > julia
> >
> >
> >
> >
> >
> >
> >
> >
> >
> > >
> > > #42: FILE: drivers/staging/qlge/qlge_devlink.c:42:
> > > +#define FILL_SEG(seg_hdr, seg_regs) \
> > > + do { \
> > > + err = qlge_fill_seg_(fmsg, &dump->seg_hdr, dump->seg_regs); \
> > > + if (err) { \
> > > + kvfree(dump); \
> > > + return err; \
> > > + } \
> > > + } while (0)
> > >
> > >
> > >
> > > The fmsg parameter and the dump parameter, both
> > > in the qlge_fill_seg_() function inside the macro, where are they referenced from?
> > > Looking through the code, I cannot see global variables in relation to
> > > this. Are they command line arguments or does is thekernel set to
> > > provide them?
> > >
> > > Any assistance will be appreciated.
> > >
> > > Kind regards,
> > > Dorcas
> > >
> > >
>
prev parent reply other threads:[~2023-10-24 12:41 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-13 6:32 [Outreachy]Clarification on code section in qlge driver Dorcas Litunya
2023-10-13 6:41 ` Julia Lawall
2023-10-13 6:52 ` Dorcas Litunya
2023-10-24 12:41 ` Julia Lawall [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4642349f-c683-e5b-7be8-375490efd6@inria.fr \
--to=julia.lawall@inria.fr \
--cc=anonolitunya@gmail.com \
--cc=outreachy@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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 read-only IMAP folder(s) and NNTP newsgroup(s).