[PATCH v2] ALSA: core/seq: Optimize the return logic in cc_ev_to_ump_midi2

songxiebing posted 1 patch 1 week, 1 day ago
sound/core/seq/seq_ump_convert.c | 35 +++++++++++++++++++-------------
1 file changed, 21 insertions(+), 14 deletions(-)
[PATCH v2] ALSA: core/seq: Optimize the return logic in cc_ev_to_ump_midi2
Posted by songxiebing 1 week, 1 day ago
There are multiple early return branches within the func, and compiler
optimizations(such as -O2/-O3)lead to abnormal stack frame analysis -
objtool cannot comfirm that the stack frames of all branches can be
correctly restored, thus generating false warnings.

Below:
>> sound/core/seq/seq_ump_convert.o: warning: objtool: cc_ev_to_ump_midi2+0x589: return with modified stack frame

So we modify it by uniformly returning at the and of the function.

Signed-off-by: songxiebing <songxiebing@kylinos.cn>
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202503200535.J3hAvcjw-lkp@intel.com/
---
v2:
- fix warning: this statement may fall through [-Wimplicit-fallthrough=]
---
 sound/core/seq/seq_ump_convert.c | 35 +++++++++++++++++++-------------
 1 file changed, 21 insertions(+), 14 deletions(-)

diff --git a/sound/core/seq/seq_ump_convert.c b/sound/core/seq/seq_ump_convert.c
index db2f169cae11..ff4ee26adad1 100644
--- a/sound/core/seq/seq_ump_convert.c
+++ b/sound/core/seq/seq_ump_convert.c
@@ -841,7 +841,7 @@ static int cc_ev_to_ump_midi2(const struct snd_seq_event *event,
 	unsigned char index = event->data.control.param & 0x7f;
 	unsigned char val = event->data.control.value & 0x7f;
 	struct ump_cvt_to_ump_bank *cc = &dest_port->midi2_bank[channel];
-	int ret;
+	int ret = 0;
 
 	/* process special CC's (bank/rpn/nrpn) */
 	switch (index) {
@@ -851,47 +851,54 @@ static int cc_ev_to_ump_midi2(const struct snd_seq_event *event,
 		cc->cc_rpn_msb = val;
 		if (cc->cc_rpn_msb == 0x7f && cc->cc_rpn_lsb == 0x7f)
 			reset_rpn(cc);
-		return ret;
+		break;
 	case UMP_CC_RPN_LSB:
 		ret = fill_rpn(cc, data, channel, true);
 		cc->rpn_set = 1;
 		cc->cc_rpn_lsb = val;
 		if (cc->cc_rpn_msb == 0x7f && cc->cc_rpn_lsb == 0x7f)
 			reset_rpn(cc);
-		return ret;
+		break;
 	case UMP_CC_NRPN_MSB:
 		ret = fill_rpn(cc, data, channel, true);
 		cc->nrpn_set = 1;
 		cc->cc_nrpn_msb = val;
-		return ret;
+		break;
 	case UMP_CC_NRPN_LSB:
 		ret = fill_rpn(cc, data, channel, true);
 		cc->nrpn_set = 1;
 		cc->cc_nrpn_lsb = val;
-		return ret;
+		break;
 	case UMP_CC_DATA:
 		cc->cc_data_msb_set = 1;
 		cc->cc_data_msb = val;
-		return fill_rpn(cc, data, channel, false);
+		ret = fill_rpn(cc, data, channel, false);
+		break;
 	case UMP_CC_BANK_SELECT:
 		cc->bank_set = 1;
 		cc->cc_bank_msb = val;
-		return 0; // skip
+		ret = 0; // skip
+		break;
 	case UMP_CC_BANK_SELECT_LSB:
 		cc->bank_set = 1;
 		cc->cc_bank_lsb = val;
-		return 0; // skip
+		ret = 0; // skip
+		break;
 	case UMP_CC_DATA_LSB:
 		cc->cc_data_lsb_set = 1;
 		cc->cc_data_lsb = val;
-		return fill_rpn(cc, data, channel, false);
+		ret = fill_rpn(cc, data, channel, false);
+		break;
+	default:
+		data->cc.status = status;
+		data->cc.channel = channel;
+		data->cc.index = index;
+		data->cc.data = upscale_7_to_32bit(event->data.control.value & 0x7f);
+		ret = 1;
+		break;
 	}
 
-	data->cc.status = status;
-	data->cc.channel = channel;
-	data->cc.index = index;
-	data->cc.data = upscale_7_to_32bit(event->data.control.value & 0x7f);
-	return 1;
+	return ret;
 }
 
 /* convert one-parameter control event to MIDI 2.0 UMP */
-- 
2.25.1
Re: [PATCH v2] ALSA: core/seq: Optimize the return logic in cc_ev_to_ump_midi2
Posted by Takashi Iwai 6 days, 13 hours ago
On Wed, 25 Mar 2026 02:51:19 +0100,
songxiebing wrote:
> 
> There are multiple early return branches within the func, and compiler
> optimizations(such as -O2/-O3)lead to abnormal stack frame analysis -
> objtool cannot comfirm that the stack frames of all branches can be
> correctly restored, thus generating false warnings.
> 
> Below:
> >> sound/core/seq/seq_ump_convert.o: warning: objtool: cc_ev_to_ump_midi2+0x589: return with modified stack frame
> 
> So we modify it by uniformly returning at the and of the function.
> 
> Signed-off-by: songxiebing <songxiebing@kylinos.cn>
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202503200535.J3hAvcjw-lkp@intel.com/
> ---
> v2:
> - fix warning: this statement may fall through [-Wimplicit-fallthrough=]

Basically I don't like to take this kind of change just because a
stupid compiler can't handle it well.  But this particular code change
itself is simple enough, so maybe still worth to take.

Applied to for-next branch now.  Thanks.


Takashi