Linux Sound subsystem development
 help / color / mirror / Atom feed
From: "Troy Mitchell" <troy.mitchell@linux.spacemit.com>
To: "Bui Duc Phuc" <phucduc.bui@gmail.com>,
	"Yixun Lan" <dlan@kernel.org>, "Takashi Iwai" <tiwai@suse.com>,
	"Mark Brown" <broonie@kernel.org>,
	"Jaroslav Kysela" <perex@perex.cz>,
	"Liam Girdwood" <lgirdwood@gmail.com>
Cc: "Troy Mitchell" <troy.mitchell@linux.spacemit.com>,
	"Goko Mell" <goku.sonxin626@gmail.com>,
	"Jinmei Wei" <weijinmei@linux.spacemit.com>,
	"Kuninori Morimoto" <kuninori.morimoto.gx@renesas.com>,
	<linux-riscv@lists.infradead.org>, <spacemit@lists.linux.dev>,
	<linux-sound@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 2/2] ASoC: spacemit: init *dp to NULL before error paths
Date: Sun, 02 Aug 2026 23:37:45 -0700	[thread overview]
Message-ID: <DKF3O1FTBV9M.M82B257B0WI2@linux.spacemit.com> (raw)
In-Reply-To: <CAABR9nEqZG1Vywb2CkoLVgqoLVPKqfYt01t3dF=WTu9qJSS=bw@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1013 bytes --]

> My intention was to make the API a bit more defensive. While the current
> implementation only has one failure path, spacemit_i2s_init_dai() may
> grow additional error paths in the future. Initializing *dp to NULL
> ensures it is left in a well-defined state on any failure.
>
> It would also avoid leaving dai uninitialized if a future caller
> accidentally skipped checking the return value before using it.

I still do not think this initialization is necessary. Currently,
spacemit_i2s_init_dai() has exactly one failure path: devm_kmemdup() fails
and the function returns -ENOMEM. The sole caller checks that return value
and exits immediately.

A caller that continued after ignoring the error would itself be incorrect
and should not be accommodated. The helper is also static and has only this
one caller, so there is no current API contract that requires the output to
be initialized on failure.

I suggest dropping this patch.

                                            - Troy

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 248 bytes --]

  parent reply	other threads:[~2026-08-03  6:38 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-31 10:15 [PATCH 1/2] ASoC: spacemit: Drop redundant error messages phucduc.bui
2026-07-31 10:15 ` [PATCH 2/2] ASoC: spacemit: init *dp to NULL before error paths phucduc.bui
2026-08-03  2:29   ` Troy Mitchell
2026-08-03  4:05     ` Bui Duc Phuc
2026-08-03  4:07       ` Bui Duc Phuc
2026-08-03  6:38         ` Troy Mitchell
2026-08-04  3:41           ` Bui Duc Phuc
2026-08-03  6:37       ` Troy Mitchell [this message]
2026-08-04  3:38         ` Bui Duc Phuc
2026-08-03  2:30 ` [PATCH 1/2] ASoC: spacemit: Drop redundant error messages Troy Mitchell
2026-08-03 18:37 ` (subset) " Mark Brown

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=DKF3O1FTBV9M.M82B257B0WI2@linux.spacemit.com \
    --to=troy.mitchell@linux.spacemit.com \
    --cc=broonie@kernel.org \
    --cc=dlan@kernel.org \
    --cc=goku.sonxin626@gmail.com \
    --cc=kuninori.morimoto.gx@renesas.com \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=linux-sound@vger.kernel.org \
    --cc=perex@perex.cz \
    --cc=phucduc.bui@gmail.com \
    --cc=spacemit@lists.linux.dev \
    --cc=tiwai@suse.com \
    --cc=weijinmei@linux.spacemit.com \
    /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