Thread (7 messages) 7 messages, 2 authors, 2026-03-19

Re: [PATCH net-next v5 2/2] net: dsa: mxl862xx: implement bridge offloading

From: Daniel Golle <daniel@makrotopia.org>
Date: 2026-03-19 11:56:17
Also in: lkml

On Thu, Mar 19, 2026 at 09:51:40AM +0100, Paolo Abeni wrote:
On 3/15/26 5:20 PM, Daniel Golle wrote:
quoted
+static int mxl862xx_set_ageing_time(struct dsa_switch *ds, unsigned int msecs)
+{
+	struct mxl862xx_cfg param;
+	int ret;
+
+	ret = MXL862XX_API_READ(ds->priv, MXL862XX_COMMON_CFGGET, param);
AI review says:

Is the param struct initialized here? Looking at MXL862XX_API_READ(), it
expands to mxl862xx_api_wrap() which writes the contents of the data buffer
to firmware MDIO registers before sending the GET command. This would send
uninitialized kernel stack data to the firmware hardware.

Every other MXL862XX_API_READ() call site in the driver uses a
zero-initialized struct. For example, in mxl862xx_setup():

	struct mxl862xx_cfg cfg = {};
	...
	ret = MXL862XX_API_READ(priv, MXL862XX_COMMON_CFGGET, cfg);

Should param be initialized with = {} here?
While I agree that just sending over uninitialized memory is kinda ugly,
for this specific firmware API call it is not a problem because it
completely discards what ever is sent, and fully overwrites all
struct members before sending it back to the host.

Anyway zero-initializing it with {} is probably still good practise
and I'll do that, so AI or static analysis tools don't complain about
that.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help