* [bug report] ALSA: ump: Add legacy raw MIDI support
@ 2023-05-25 7:57 Dan Carpenter
2023-05-25 8:29 ` Takashi Iwai
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2023-05-25 7:57 UTC (permalink / raw)
To: tiwai; +Cc: alsa-devel
Hello Takashi Iwai,
The patch 0b5288f5fe63: "ALSA: ump: Add legacy raw MIDI support" from
May 23, 2023, leads to the following Smatch static checker warning:
sound/core/ump_convert.c:343 cvt_legacy_cmd_to_ump()
warn: duplicate check 'buf[2]' (previous on line 333)
sound/core/ump_convert.c
305 static int cvt_legacy_cmd_to_ump(struct snd_ump_endpoint *ump,
306 struct ump_cvt_to_ump *cvt,
307 unsigned char group, u32 *data,
308 unsigned char bytes)
309 {
310 const unsigned char *buf = cvt->buf;
311 struct ump_cvt_to_ump_bank *cc;
312 union snd_ump_midi2_msg *midi2 = (union snd_ump_midi2_msg *)data;
313 unsigned char status, channel;
314
315 BUILD_BUG_ON(sizeof(union snd_ump_midi1_msg) != 4);
316 BUILD_BUG_ON(sizeof(union snd_ump_midi2_msg) != 8);
317
318 /* for MIDI 1.0 UMP, it's easy, just pack it into UMP */
319 if (ump->info.protocol & SNDRV_UMP_EP_INFO_PROTO_MIDI1) {
320 data[0] = ump_compose(UMP_MSG_TYPE_MIDI1_CHANNEL_VOICE,
321 group, 0, buf[0]);
322 data[0] |= buf[1] << 8;
323 if (bytes > 2)
324 data[0] |= buf[2];
325 return 4;
326 }
327
328 status = *buf >> 4;
329 channel = *buf & 0x0f;
330 cc = &cvt->bank[channel];
331
332 /* special handling: treat note-on with 0 velocity as note-off */
333 if (status == UMP_MSG_STATUS_NOTE_ON && !buf[2])
334 status = UMP_MSG_STATUS_NOTE_OFF;
This if statment
335
336 /* initialize the packet */
337 data[0] = ump_compose(UMP_MSG_TYPE_MIDI2_CHANNEL_VOICE,
338 group, status, channel);
339 data[1] = 0;
340
341 switch (status) {
342 case UMP_MSG_STATUS_NOTE_ON:
--> 343 if (!buf[2])
344 status = UMP_MSG_STATUS_NOTE_OFF;
and this one are the same. One could be deleted.
345 fallthrough;
346 case UMP_MSG_STATUS_NOTE_OFF:
347 midi2->note.note = buf[1];
348 midi2->note.velocity = upscale_7_to_16bit(buf[2]);
349 break;
350 case UMP_MSG_STATUS_POLY_PRESSURE:
351 midi2->paf.note = buf[1];
352 midi2->paf.data = upscale_7_to_32bit(buf[2]);
353 break;
354 case UMP_MSG_STATUS_CC:
355 switch (buf[1]) {
356 case UMP_CC_RPN_MSB:
357 cc->rpn_set = 1;
358 cc->cc_rpn_msb = buf[2];
359 return 0; // skip
360 case UMP_CC_RPN_LSB:
361 cc->rpn_set = 1;
362 cc->cc_rpn_lsb = buf[2];
363 return 0; // skip
364 case UMP_CC_NRPN_MSB:
365 cc->nrpn_set = 1;
366 cc->cc_nrpn_msb = buf[2];
367 return 0; // skip
368 case UMP_CC_NRPN_LSB:
369 cc->nrpn_set = 1;
370 cc->cc_nrpn_lsb = buf[2];
371 return 0; // skip
372 case UMP_CC_DATA:
373 cc->cc_data_msb = buf[2];
374 return 0; // skip
375 case UMP_CC_BANK_SELECT:
376 cc->bank_set = 1;
377 cc->cc_bank_msb = buf[2];
378 return 0; // skip
379 case UMP_CC_BANK_SELECT_LSB:
380 cc->bank_set = 1;
381 cc->cc_bank_lsb = buf[2];
382 return 0; // skip
383 case UMP_CC_DATA_LSB:
384 cc->cc_data_lsb = buf[2];
385 if (cc->rpn_set || cc->nrpn_set)
386 fill_rpn(cc, midi2);
387 else
388 return 0; // skip
389 break;
390 default:
391 midi2->cc.index = buf[1];
392 midi2->cc.data = upscale_7_to_32bit(buf[2]);
393 break;
394 }
395 break;
396 case UMP_MSG_STATUS_PROGRAM:
397 midi2->pg.program = buf[1];
398 if (cc->bank_set) {
399 midi2->pg.bank_valid = 1;
400 midi2->pg.bank_msb = cc->cc_bank_msb;
401 midi2->pg.bank_lsb = cc->cc_bank_lsb;
402 cc->bank_set = 0;
403 cc->cc_bank_msb = cc->cc_bank_lsb = 0;
404 }
405 break;
406 case UMP_MSG_STATUS_CHANNEL_PRESSURE:
407 midi2->caf.data = upscale_7_to_32bit(buf[1]);
408 break;
409 case UMP_MSG_STATUS_PITCH_BEND:
410 midi2->pb.data = upscale_14_to_32bit(buf[1] | (buf[2] << 7));
411 break;
412 default:
413 return 0;
414 }
415
416 return 8;
417 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread
* [bug report] ALSA: ump: Add legacy raw MIDI support
@ 2023-05-25 8:00 Dan Carpenter
2023-05-25 8:29 ` Takashi Iwai
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2023-05-25 8:00 UTC (permalink / raw)
To: tiwai; +Cc: alsa-devel
Hello Takashi Iwai,
The patch 0b5288f5fe63: "ALSA: ump: Add legacy raw MIDI support" from
May 23, 2023, leads to the following Smatch static checker warning:
sound/core/ump_convert.c:460 do_convert_to_ump()
warn: right shifting more than type allows 8 vs 8
sound/core/ump_convert.c
419 static int do_convert_to_ump(struct snd_ump_endpoint *ump,
420 unsigned char group, unsigned char c, u32 *data)
421 {
422 /* bytes for 0x80-0xf0 */
423 static unsigned char cmd_bytes[8] = {
424 3, 3, 3, 3, 2, 2, 3, 0
425 };
426 /* bytes for 0xf0-0xff */
427 static unsigned char system_bytes[16] = {
428 0, 2, 3, 2, 0, 0, 1, 0, 1, 1, 1, 1, 0, 0, 1, 1
429 };
430 struct ump_cvt_to_ump *cvt = &ump->out_cvts[group];
431 unsigned char bytes;
432
433 if (c == UMP_MIDI1_MSG_SYSEX_START) {
434 cvt->in_sysex = 1;
435 cvt->len = 0;
436 return 0;
437 }
438 if (c == UMP_MIDI1_MSG_SYSEX_END) {
439 if (!cvt->in_sysex)
440 return 0; /* skip */
441 return cvt_legacy_sysex_to_ump(cvt, group, data, true);
442 }
443
444 if ((c & 0xf0) == UMP_MIDI1_MSG_REALTIME) {
445 bytes = system_bytes[c & 0x0f];
446 if (!bytes)
447 return 0; /* skip */
448 if (bytes == 1) {
449 data[0] = ump_compose(UMP_MSG_TYPE_SYSTEM, group, 0, c);
450 return 4;
451 }
452 cvt->buf[0] = c;
453 cvt->len = 1;
454 cvt->cmd_bytes = bytes;
455 cvt->in_sysex = 0; /* abort SysEx */
456 return 0;
457 }
458
459 if (c & 0x80) {
--> 460 bytes = cmd_bytes[(c >> 8) & 7];
^^^^^^
Based on the if statement, it looks like c >> 4 was intended.
461 cvt->buf[0] = c;
462 cvt->len = 1;
463 cvt->cmd_bytes = bytes;
464 cvt->in_sysex = 0; /* abort SysEx */
465 return 0;
466 }
467
468 if (cvt->in_sysex) {
469 cvt->buf[cvt->len++] = c;
470 if (cvt->len == 6)
471 return cvt_legacy_sysex_to_ump(cvt, group, data, false);
472 return 0;
473 }
474
475 if (!cvt->len)
476 return 0;
477
478 cvt->buf[cvt->len++] = c;
479 if (cvt->len < cvt->cmd_bytes)
480 return 0;
481 cvt->len = 1;
482 if ((cvt->buf[0] & 0xf0) == UMP_MIDI1_MSG_REALTIME)
483 return cvt_legacy_system_to_ump(cvt, group, data);
484 return cvt_legacy_cmd_to_ump(ump, cvt, group, data, cvt->cmd_bytes);
485 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [bug report] ALSA: ump: Add legacy raw MIDI support
2023-05-25 7:57 Dan Carpenter
@ 2023-05-25 8:29 ` Takashi Iwai
0 siblings, 0 replies; 4+ messages in thread
From: Takashi Iwai @ 2023-05-25 8:29 UTC (permalink / raw)
To: Dan Carpenter; +Cc: alsa-devel
On Thu, 25 May 2023 09:57:47 +0200,
Dan Carpenter wrote:
>
> Hello Takashi Iwai,
>
> The patch 0b5288f5fe63: "ALSA: ump: Add legacy raw MIDI support" from
> May 23, 2023, leads to the following Smatch static checker warning:
>
> sound/core/ump_convert.c:343 cvt_legacy_cmd_to_ump()
> warn: duplicate check 'buf[2]' (previous on line 333)
>
> sound/core/ump_convert.c
> 305 static int cvt_legacy_cmd_to_ump(struct snd_ump_endpoint *ump,
> 306 struct ump_cvt_to_ump *cvt,
> 307 unsigned char group, u32 *data,
> 308 unsigned char bytes)
> 309 {
> 310 const unsigned char *buf = cvt->buf;
> 311 struct ump_cvt_to_ump_bank *cc;
> 312 union snd_ump_midi2_msg *midi2 = (union snd_ump_midi2_msg *)data;
> 313 unsigned char status, channel;
> 314
> 315 BUILD_BUG_ON(sizeof(union snd_ump_midi1_msg) != 4);
> 316 BUILD_BUG_ON(sizeof(union snd_ump_midi2_msg) != 8);
> 317
> 318 /* for MIDI 1.0 UMP, it's easy, just pack it into UMP */
> 319 if (ump->info.protocol & SNDRV_UMP_EP_INFO_PROTO_MIDI1) {
> 320 data[0] = ump_compose(UMP_MSG_TYPE_MIDI1_CHANNEL_VOICE,
> 321 group, 0, buf[0]);
> 322 data[0] |= buf[1] << 8;
> 323 if (bytes > 2)
> 324 data[0] |= buf[2];
> 325 return 4;
> 326 }
> 327
> 328 status = *buf >> 4;
> 329 channel = *buf & 0x0f;
> 330 cc = &cvt->bank[channel];
> 331
> 332 /* special handling: treat note-on with 0 velocity as note-off */
> 333 if (status == UMP_MSG_STATUS_NOTE_ON && !buf[2])
> 334 status = UMP_MSG_STATUS_NOTE_OFF;
>
> This if statment
>
> 335
> 336 /* initialize the packet */
> 337 data[0] = ump_compose(UMP_MSG_TYPE_MIDI2_CHANNEL_VOICE,
> 338 group, status, channel);
> 339 data[1] = 0;
> 340
> 341 switch (status) {
> 342 case UMP_MSG_STATUS_NOTE_ON:
> --> 343 if (!buf[2])
> 344 status = UMP_MSG_STATUS_NOTE_OFF;
>
> and this one are the same. One could be deleted.
Correct. I'll submit a fix patch.
Thanks!
Takashi
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [bug report] ALSA: ump: Add legacy raw MIDI support
2023-05-25 8:00 [bug report] ALSA: ump: Add legacy raw MIDI support Dan Carpenter
@ 2023-05-25 8:29 ` Takashi Iwai
0 siblings, 0 replies; 4+ messages in thread
From: Takashi Iwai @ 2023-05-25 8:29 UTC (permalink / raw)
To: Dan Carpenter; +Cc: alsa-devel
On Thu, 25 May 2023 10:00:10 +0200,
Dan Carpenter wrote:
>
> Hello Takashi Iwai,
>
> The patch 0b5288f5fe63: "ALSA: ump: Add legacy raw MIDI support" from
> May 23, 2023, leads to the following Smatch static checker warning:
>
> sound/core/ump_convert.c:460 do_convert_to_ump()
> warn: right shifting more than type allows 8 vs 8
>
> sound/core/ump_convert.c
> 419 static int do_convert_to_ump(struct snd_ump_endpoint *ump,
> 420 unsigned char group, unsigned char c, u32 *data)
> 421 {
> 422 /* bytes for 0x80-0xf0 */
> 423 static unsigned char cmd_bytes[8] = {
> 424 3, 3, 3, 3, 2, 2, 3, 0
> 425 };
> 426 /* bytes for 0xf0-0xff */
> 427 static unsigned char system_bytes[16] = {
> 428 0, 2, 3, 2, 0, 0, 1, 0, 1, 1, 1, 1, 0, 0, 1, 1
> 429 };
> 430 struct ump_cvt_to_ump *cvt = &ump->out_cvts[group];
> 431 unsigned char bytes;
> 432
> 433 if (c == UMP_MIDI1_MSG_SYSEX_START) {
> 434 cvt->in_sysex = 1;
> 435 cvt->len = 0;
> 436 return 0;
> 437 }
> 438 if (c == UMP_MIDI1_MSG_SYSEX_END) {
> 439 if (!cvt->in_sysex)
> 440 return 0; /* skip */
> 441 return cvt_legacy_sysex_to_ump(cvt, group, data, true);
> 442 }
> 443
> 444 if ((c & 0xf0) == UMP_MIDI1_MSG_REALTIME) {
> 445 bytes = system_bytes[c & 0x0f];
> 446 if (!bytes)
> 447 return 0; /* skip */
> 448 if (bytes == 1) {
> 449 data[0] = ump_compose(UMP_MSG_TYPE_SYSTEM, group, 0, c);
> 450 return 4;
> 451 }
> 452 cvt->buf[0] = c;
> 453 cvt->len = 1;
> 454 cvt->cmd_bytes = bytes;
> 455 cvt->in_sysex = 0; /* abort SysEx */
> 456 return 0;
> 457 }
> 458
> 459 if (c & 0x80) {
> --> 460 bytes = cmd_bytes[(c >> 8) & 7];
> ^^^^^^
> Based on the if statement, it looks like c >> 4 was intended.
Right, currently it's a bogus value. I'll submit the fix patch.
Thanks!
Takashi
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-05-26 11:46 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-25 8:00 [bug report] ALSA: ump: Add legacy raw MIDI support Dan Carpenter
2023-05-25 8:29 ` Takashi Iwai
-- strict thread matches above, loose matches on Subject: below --
2023-05-25 7:57 Dan Carpenter
2023-05-25 8:29 ` Takashi Iwai
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox