[espeak-ng:master] reported: Fix crash in voices.c:SetVoiceScores #github


espeak-ng@groups.io Integration <espeak-ng@...>
 

[espeak-ng:master] New Comment on Pull Request #925 Fix crash in voices.c:SetVoiceScores
By valdisvi:

I have not tested it through API, but since c13b3e534b0113c6ee9093ff9c4234e66e9a10c5 commit CLI interface supports all parameter to list all variants of the voices. Can you check it through API and adjust fix that it also checks for all string directly in the code?


espeak-ng@groups.io Integration <espeak-ng@...>
 

[espeak-ng:master] New Comment on Pull Request #925 Fix crash in voices.c:SetVoiceScores
By valdisvi:

I have not tested it through API, but since c13b3e534b0113c6ee9093ff9c4234e66e9a10c5 commit CLI interface supports all parameter to list all variants of the voices. Can you check it through API and adjust fix that it also checks for all string directly in the code? So, my understanding is, that if we don't need backward compatibility, that section of code could be deleted at all and proposed fix is only to allow 'old' behaviour of API calls.


espeak-ng@groups.io Integration <espeak-ng@...>
 

[espeak-ng:master] New Comment on Pull Request #925 Fix crash in voices.c:SetVoiceScores
By rhdunn:

Retaining backward compatibility in the API is one of the general aims of espeak-ng, as this allows clients programmed against the original espeak API to work.

It would be useful to update the test/api.c test code to ensure that this usage continues to work -- specifically, not crash.


espeak-ng@groups.io Integration <espeak-ng@...>
 

[espeak-ng:master] New Comment on Pull Request #925 Fix crash in voices.c:SetVoiceScores
By jaacoppi:

Since the commit is clearly a faulty attempt in code cleanup a simple git revert should be enough. Unfortunately I have limited time to investigate this for a better solution at the moment.

Adding a test case is a very good idea. Is the API call documented in enough detail to know which cases should be tested?


espeak-ng@groups.io Integration <espeak-ng@...>
 

[espeak-ng:master] New Comment on Pull Request #925 Fix crash in voices.c:SetVoiceScores
By jbowler:

Passing the string "all" from application code will probably cause a crash with older versions of espeak-ng, even though it will compile. The problem is that "all" is language code "ll" (which I admit doesn't exist) with a priority of 97, however there is only a single terminating \0; "all\0" should be used for safety and, even then, it will have the opposite to the desired effect on older builds. This is assuming I understand the syntax of espeak_ListVoices; the only documentation I could find was in the header file and the documented behavior is somewhat nightmarish (the same structure with two different string formats!)

I only need to list all the voices including the mbrola ones. I find NULL intuitively obvious because the identifier and name fields work the same way with NULL. I didn't read the source code to end up with my solution in 2019; I noticed a NULL parameter to espeak_ListVoices did not do what I expected (it drops the mbrola voices) so I experimented with NULL for a language and dropped the variants in my own code.

globbing strings are the other obvious approach, e.g "\0en-us\0aen\0ben-\0z-us\0\377\0", but obviously that's a lot more work to implement and it still apparently has the problem of skipping the mbrola voices though I guess a "" at the start of the string could say "match mbrola too".