Ticket #227 (closed defect: fixed)

Opened 3 years ago

Last modified 20 months ago

[PATCH] Improvements to padsp to fix various issues (e.g. amsn crash)

Reported by: coling Owned by: lennart
Milestone: Component: daemon
Keywords: Cc:

Description

Originally reported via Mandriva Bugzilla.

It seems that some apps have issues with the way the functions overridden in padsp operate. e.g. amsn crashes.

In the code, there are some comments about how Firefox needs this response to not crash etc. It seems to me that the more sensible option is to rely on the original method in all cases that we cannot handle and thus any special handling like this should not be required.

This patch makes these changes and also fixes some undefined behaviour when compiling with non-glibc backends (due to the fact that vsprintf may not handle NULLs nicely - see http://gcc.gnu.org/bugzilla/show_bug.cgi?id=25609).

In our testing it fixes issues in amsn and does not have any adverse affect on firefox when testing.

This could be related to #27.

Change History

  Changed 3 years ago by coling

I should also say that if the patch is committed, credits should be to myself (Colin Guthrie) and Gustavo De Nardin. Thanks.

  Changed 2 years ago by coling

  • summary changed from [PATCH] Improvements to paoss to fix various issues (e.g. amsn crash) to [PATCH] Improvements to padsp to fix various issues (e.g. amsn crash)

  Changed 2 years ago by lennart

Uh, the non-glibc issue doesn't really count, since padsp is very glibc-specific anyway.

  Changed 2 years ago by lennart

  • status changed from new to closed
  • resolution set to fixed

(In [2106]) Improve compatibility with applications which like to pass invalid strings to the libc functions we overwrite, by handing directly to the original function. Patch by Colin Guthrie and Gustavo De Nardin, Closes #227

  Changed 2 years ago by gustavodn

I know Pulseaudio has been ported at least to FreeBSD (non-glibc system), and padsp is installed there. Also, another good thing of Pulseaudio would be helping the unix audio world by being a stable ground for audio across unix platforms, maybe replacing the OSS api on that function, so I believe portability is quite important. There's already badness on this topic, with ALSA being Linux specific and apps using its API directly, giving a hard time for non-Linux users.

FWIW, passing NULL filename to fopen() seems valid to me, the standard just says it must return NULL in case of failure.

Thanks for applying the patch. cya

follow-up: ↓ 7   Changed 2 years ago by lennart

Uh, I am pretty sure the newest version of PA doesn't run on FreeBSD unchanged.

I personally don't care much about non-Linux platforms. I'm not going to make things extra difficult for FreeBSD and other Unix porters, but OTOH I am not going to limit myself just because they fancy an exotic OS. I only care about Linux, but I am happy to merge all (sensible) patches other people send me, including those necessary for portability.

Oh, and the ALSA libs can run on top of OSS. So the "ALSA is not portable" argument doesn't hold.

Portability is not important -- at least not to *BSD. It's not more than a favour to people with a fancy for exotic OSes.

in reply to: ↑ 6 ; follow-up: ↓ 8   Changed 2 years ago by gustavodn

Replying to lennart:

Uh, I am pretty sure the newest version of PA doesn't run on FreeBSD unchanged.

So why is it listed in AboutPulseAudio#SupportedOperatingSystems? (Along with Solaris and Win32...) Is that outdated?

Oh, and the ALSA libs can run on top of OSS. So the "ALSA is not portable" argument doesn't hold.

Don't know, I know for-ALSA stuff made things hard for FreeBSD when I used it on desktop, years ago, maybe things changed.

Portability is not important -- at least not to *BSD. It's not more than a favour to people with a fancy for exotic OSes.

Amusing statement... I'm gonna bookmark this ticket.

in reply to: ↑ 7   Changed 2 years ago by lennart

Replying to gustavodn:

Replying to lennart:

Uh, I am pretty sure the newest version of PA doesn't run on FreeBSD unchanged.

So why is it listed in AboutPulseAudio#SupportedOperatingSystems? (Along with Solaris and Win32...) Is that outdated?

While the newest version of PA probably doesn't compile on FreeBSD out-of-the-box some not-that-old version did. It's all a matter of supplying me with patches. Most likely getting PA work on FreeBSD again is relatively little work. However, I am not doing that work and am relying on third party patches. When I do a new PA release I don't want to wait until I know every single port works fine. Linux is the only port that matters.

Note: See TracTickets for help on using tickets.