lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZBOfuSBifFO7O/xQ@shell.armlinux.org.uk>
Date:   Thu, 16 Mar 2023 23:01:13 +0000
From:   "Russell King (Oracle)" <linux@...linux.org.uk>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     Andrew Halaney <ahalaney@...hat.com>, linux-kernel@...r.kernel.org,
        agross@...nel.org, andersson@...nel.org, konrad.dybcio@...aro.org,
        davem@...emloft.net, edumazet@...gle.com, pabeni@...hat.com,
        robh+dt@...nel.org, krzysztof.kozlowski+dt@...aro.org,
        vkoul@...nel.org, bhupesh.sharma@...aro.org,
        mturquette@...libre.com, sboyd@...nel.org, peppe.cavallaro@...com,
        alexandre.torgue@...s.st.com, joabreu@...opsys.com,
        mcoquelin.stm32@...il.com, richardcochran@...il.com,
        veekhee@...le.com, tee.min.tan@...ux.intel.com,
        mohammad.athari.ismail@...el.com, jonathanh@...dia.com,
        ruppala@...dia.com, bmasney@...hat.com,
        andrey.konovalov@...aro.org, linux-arm-msm@...r.kernel.org,
        netdev@...r.kernel.org, devicetree@...r.kernel.org,
        linux-clk@...r.kernel.org,
        linux-stm32@...md-mailman.stormreply.com,
        linux-arm-kernel@...ts.infradead.org, ncai@...cinc.com,
        jsuraj@....qualcomm.com, hisunil@...cinc.com
Subject: Re: [PATCH net-next 08/11] net: stmmac: Add EMAC3 variant of dwmac4

On Thu, Mar 16, 2023 at 11:52:34AM -0700, Jakub Kicinski wrote:
> On Thu, 16 Mar 2023 13:36:09 -0500 Andrew Halaney wrote:
> > static void emac3_config_cbs(struct mac_device_info *hw, u32 send_slope,
> > 				    u32 idle_slope, u32 high_credit,
> > 				    u32 low_credit, u32 queue)
> > 
> > I agree, that's quite gnarly to read. the emac3_config_cbs is the
> > callback, so it's already at 6 arguments, so there's nothing I can
> > trim there. I could create some struct for readability, populate that,
> > then call the do_config_cbs() func with it from emac3_config_cbs.
> > Is that the sort of thing you want to see?
> 
> Yes, a structure is much better, because it can be initialized member
> by member,
> 
> struct bla my_bla = { .this = 1, .that = 2, .and = 3, another = 4, };
> 
> That's much easier to read. A poor man's version of Python's keyword
> arguments, if you will.

What I would say is be careful with that - make sure "struct bla" is
specific to the interface being called and not generic.

I had that mistake with struct phylink_state... and there is an
endless stream of people who don't seem to bother reading the
documentation, who blindly access whatever members of that they
damn well please because it suits them, even when either they
shouldn't be writing to them, or when phylink doesn't guarantee
their contents, they read them.

As a result, I'm now of the opinion that using a struct to pass
arguments is in principle a bad idea.

There's other reasons why it's a bad idea. Many ABIs are capable of
passing arguments to functions via processor registers. As soon as
one uses a struct, they typically end up being written to memory.
Not only does that potentially cause cache line churn, it also
means that there could be more slow memory accesses that have to be
made at some point, potentially making other accesses slow.

So, all in all, I'm really not a fan of the struct approach for
all the reasons above.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ