Re: Proposal: replacing esound with polypaudio in 2.10

[Alan Cox <alan lxorguk ukuu org uk>]

On Gwe, 2004-10-29 at 00:02, Lennart Poettering wrote:

> > That would make sense - it's just another plugin after all.
> You're right. Could you tell me what format this meta data should have?
> anything more than just a bunch of tuples [timestamp, text]?

I would assume thats sufficient but perhaps an accessibility person can
provide more insight (text being utf-8 format of course)

> Tsss. You picked an interesting piece of code, really. Would
> commenting the function suffice your needs? 

It would help yes

> pa_strbuf uses snprintf internally. So it fails under the same conditions
> as snprintf.

That isnt actually true. Because you didnt use glib you re-created a bug
in glib on 64bit systems. Your sizing is done with "int" and without
overflow checks so you fail on strings like
	"%*s", 1<<32+4, "Bang"

Now auditing so far I can find no problems with that case, because it
can't happen any way I can find but it illustrates the perils of not
reusing code.

> specify a non-standard configuration file if you like. Polypaudio just
> looks for the configuration script in $HOME first, in case it is not

If $HOME isn't set most applications then use getpwuid(getuid()) to find
the home directory and try that - its an 'expected' fallback.

> Yes, you're right on that, but only when locating the configuration
> file to use. So it ain't a security hole. Who edits his configuration
> files in the same time as he starts polypaudio anyway?

But what happens if they vanish over NFS. If your code is robust you
don't need access.

> > Other issues:
> > - You use snprintf in the signal handler - thats not guaranteed safe I
> > believe
> 
> As far as I know (f)printf() is not safe since it accesses a FILE
> object, but snprintf should be safe.

The memory allocating version isnt. I've asked Ulrich Drepper about the
general case. I'll let you know what he says

> his streams easily. If that name cannot be read, it is simple set to
> "libao". Short: it's just usef for eyecandy and is otherwise useless.

Ok

> I do believe that NODELAY is a good idea in this case. Why? It lowers the
> latency of network transmissions. Polypaudio usually writes very large
> blocks to the sockets and it usually takes some time to get the next
> block (i.e. this is controlled by the soundcard's crystal), so most of
> the time there's no point in waiting for more data.

TCP_NODELAY causes confusion because its functionality isnt always
obvious to people. TCP has a rule which is something like this
	
If I don't have a frame (typically about 1460) bytes of data
	If there is already a frame in flight
		Wait a briefly for other data before sending.

In practice this means that setting TCP_NODELAY has no useful effect 
on streaming audio and can have negative effects as it causes lots of
sequences of 1460,1460, 140, 1460,1460,140 or similar to occur and may
also trigger silly window syndrome on old boxes which harms performance.

Linux has TCP_CORK which allows you to jam a virtual cork in a
connection (like putting a cork in a wine bottle) to block up data
nicely but that isn't portable. What is portable of course is switching
NODELAY off while streaming and back on when appropriate.

> > - Trusts getaddrinfo response for address length. You need to validate
> > it is sane(eg 4 byte for IP)
> 
> Hmpf. I always trust what the libc/kernel tells me. 

libc tells you want the name server told it, and the nameserver can
reply with dumb stuff which some systems then believe. Its a common
"gotcha"

Alan