mbox series

[v4,00/10] topology: decode: Various fixes

Message ID 1598864943-22883-1-git-send-email-piotrx.maziarz@linux.intel.com
Headers show
Series topology: decode: Various fixes | expand

Message

Piotr Maziarz Aug. 31, 2020, 9:08 a.m. UTC
This series fixes various problems with topology decoding mechanism.
Some of the problems were critical like improper memory management or
infinite loops that were causing undefined behaviour or program crashes,
while other resulted in losing some data during conversion.

Bugs found while testing with Intel SST topologies.

Signed-off-by: Piotr Maziarz <piotrx.maziarz@linux.intel.com>
Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
Reviewed-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Changelog:
v2:
  -Divide into more patches, critical fixes are in separate patches now
  -More specific descriptions
  -Fix a typo UML to UCM
  -Add error reporting in topology: decode: fix channel map memory
   allocation
  -Remove goto again in topology: Make buffer for saving dynamic size
   for better readability

v3:
  -No functional changes
  -Changed UCM to more descriptive standard ALSA configuration files
  -removed Gerrit's Change-Id
  -Added missing signed-offs

v4:
  -Resend, added Reviewed-by tags

Piotr Maziarz (10):
  topology: decode: Fix channel map memory allocation
  topology: decode: Fix infinite loop in decoding enum control
  topology: decode: Remove decoding  values for enum control
  topology: decode: Add enum control texts as separate element
  topology: decode: Fix printing texts section
  topology: decode: Change declaration of enum decoding function
  topology: decode: Fix decoding PCM formats and rates
  topology: decode: Print sig_bits field in PCM capabilities section
  topology: decode: Add DAI name printing
  topology: Make buffer for saving dynamic size

 src/topology/ctl.c        | 51 ++++++++++++++++++++++-------------------------
 src/topology/dapm.c       |  3 +--
 src/topology/pcm.c        | 11 +++++++---
 src/topology/save.c       | 34 ++++++++++++++++++++++++++-----
 src/topology/text.c       |  2 +-
 src/topology/tplg_local.h |  2 +-
 6 files changed, 64 insertions(+), 39 deletions(-)

Comments

Jaroslav Kysela Aug. 31, 2020, 11:11 a.m. UTC | #1
Dne 31. 08. 20 v 11:08 Piotr Maziarz napsal(a):
> This series fixes various problems with topology decoding mechanism.
> Some of the problems were critical like improper memory management or
> infinite loops that were causing undefined behaviour or program crashes,
> while other resulted in losing some data during conversion.
> 
> Bugs found while testing with Intel SST topologies.

Thank you for this work. I applied all patches to the alsa-lib repository. I
dislike the last one - dynamic allocation for each printf(), but I applied it
until we found a better solution.

We may use the dynamic allocation only when the printf is bigger than the 1024
bytes threshold (and keep the small buffer on stack otherwise) or create 'dst'
structure which will carry the output buffer point and the temporary buffer
pointer which will be freed when the output is finished.

				Thank you,
					Jaroslav
Jaroslav Kysela Aug. 31, 2020, 5:57 p.m. UTC | #2
Dne 31. 08. 20 v 13:11 Jaroslav Kysela napsal(a):
> Dne 31. 08. 20 v 11:08 Piotr Maziarz napsal(a):
>> This series fixes various problems with topology decoding mechanism.
>> Some of the problems were critical like improper memory management or
>> infinite loops that were causing undefined behaviour or program crashes,
>> while other resulted in losing some data during conversion.
>>
>> Bugs found while testing with Intel SST topologies.
> 
> Thank you for this work. I applied all patches to the alsa-lib repository. I
> dislike the last one - dynamic allocation for each printf(), but I applied it
> until we found a better solution.
> 
> We may use the dynamic allocation only when the printf is bigger than the 1024
> bytes threshold (and keep the small buffer on stack otherwise) or create 'dst'
> structure which will carry the output buffer point and the temporary buffer
> pointer which will be freed when the output is finished.

I implemented the later suggestion in:

https://github.com/alsa-project/alsa-lib/commit/472ab5db67a0ed04de634214773e7b17d10b5415

There are also other fixes in the topology library. It would be nice, if you
can give a test.

			Thank you,
				Jaroslav
Piotr Maziarz Sept. 3, 2020, 8:10 a.m. UTC | #3
On 2020-08-31 19:57, Jaroslav Kysela wrote:
> Dne 31. 08. 20 v 13:11 Jaroslav Kysela napsal(a):
>> Dne 31. 08. 20 v 11:08 Piotr Maziarz napsal(a):
>>> This series fixes various problems with topology decoding mechanism.
>>> Some of the problems were critical like improper memory management or
>>> infinite loops that were causing undefined behaviour or program crashes,
>>> while other resulted in losing some data during conversion.
>>>
>>> Bugs found while testing with Intel SST topologies.
>>
>> Thank you for this work. I applied all patches to the alsa-lib repository. I
>> dislike the last one - dynamic allocation for each printf(), but I applied it
>> until we found a better solution.
>>
>> We may use the dynamic allocation only when the printf is bigger than the 1024
>> bytes threshold (and keep the small buffer on stack otherwise) or create 'dst'
>> structure which will carry the output buffer point and the temporary buffer
>> pointer which will be freed when the output is finished.
> 
> I implemented the later suggestion in:
> 
> https://github.com/alsa-project/alsa-lib/commit/472ab5db67a0ed04de634214773e7b17d10b5415
> 
> There are also other fixes in the topology library. It would be nice, if you
> can give a test.
> 
> 			Thank you,
> 				Jaroslav
> 
I've tested your changes on a bunch of our topologies, everything works 
fine.
Thanks for merge and update.

Best Regards,
Piotr