[FFmpeg-devel] [PATCH] RTSP alternate protocol 2-3/3

[Michael Niedermayer]

On Sat, Mar 01, 2008 at 10:07:28PM -0500, Ronald S. Bultje wrote:
> Hi Michael,
> 
> On Sat, Mar 1, 2008 at 9:49 PM, Michael Niedermayer <michaelni at gmx.at>
> wrote:
> 
> > > That's on purpose. I try to make the "reindent" patch as small as
> > possible
> > > so I indent the new lines correctly already and only leave the lines
> > that
> > > this patch does not really touch in the old indent (and then the
> > reindent
> > > patch, 4/4, fixes those lines).
> >
> > how can the following end up with correct indention with no reindent?
> > These are new lines ...
> >
> > > > > +    do {
> > > > > +        int protocol = protocol_mask & ~(protocol_mask - 1);
> > > > > +
> > > > > +    err = make_setup_request(s, host, port, protocol);
> > > > > +    if (err < 0)
> >
> 
> Because the 2 bottom lines already exist, I'm making functional changes to
> them (but not reindenting, since that should be a separate patch). The other
> lines (such as the first two here) are new, and thus I directly reindent
> them correctly. This sounds kind of weird, I now realize, so please tell me
> how you'd prefer it: 

> (1) just reindent any line I happen to touch already,

yes


> or (2) make all lines reindented wrong and reindent everything (the whole
> block) in the separate reindent patch or (3) leave it like this?
> 
> 
> > > > Also i suspect this code will leak various things.
> > >
> > > make_setup_request() doesn't allocate resources, be that memory or FDs.
> > If
> > > there's something specific that I'm missing, please be more specific.
> > :-).
> >
> > There are 2 url_open() it does not seem its guranteed for them to be
> > closed
> > but then i dont know the code ...
> 
> 
> That's closed in rtsp_close_stream(), which is called in open() when
> make_setup_request() fails (in the fail: at the bottom).

make_setup_request() fail looks like this:
fail:
    return err;
}
The calling code does not free resources allocated in make_setup_request() it
also would be wrong to free it there anyway, it just calls make_setup_request()
again overwriting whatever is already opened.

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The educated differ from the uneducated as much as the living from the
dead. -- Aristotle 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080302/1d19aba3/attachment.pgp>