[MPlayer-dev-eng] [PATCH] vstream client support

[Reimar Döffinger]

Hi,

doxygen-Style comments are missing...

> +void vstream_error(const char *format, ...) {
> +    char buf[1024];
> +    va_list va;
> +    va_start(va, format);
> +    vsnprintf(buf, 1024, format, va);

This can produce a non null-terminated string. Add e.g. buf[1023] = 0;

> +  if(!p->host) {
> +    mp_msg(MSGT_OPEN, MSGL_ERR, "We need a host name (ex: tivo://hostname/fsid)\n");
> +    m_struct_free(&stream_opts, opts);
> +    return STREAM_ERROR;

Does this have to be freed here? And if initialization succeed, when
should it be freed then? At the end of this function? Or on close?

> +  if (!strcmp(p->fsid, "list")) {
> +    vstream_list_streams(0);
> +    return STREAM_ERROR;
> +  } else if (!strcmp(p->fsid, "llist")) {
> +    vstream_list_streams(1);
> +    return STREAM_ERROR;
> +  }

Hmmm. Don't you need a m_struct_free here, too? I actually prefer gotos
to have all the freeing in one place (and only one exit point of the
function) this helps a lot avoiding such things.


> +  stream->start_pos = 0;
> +  stream->end_pos = vstream_streamsize();
> +  mp_msg(MSGT_OPEN, MSGL_DBG2, "Tivo stream size is %d\n", stream->end_pos);

Maybe better MSGL_V, I think _DBG2 is for messages that might be
printed several times or output a lot in general...

Greetings,
Reimar Döffinger