[FFmpeg-devel] [PATCH] tcp.c/udp.c memleak?

[Michael Niedermayer]

On Fri, Aug 22, 2008 at 05:32:56PM -0400, Ronald S. Bultje wrote:
> Hi,
> 
> On Thu, Aug 21, 2008 at 1:02 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > On Wed, Aug 20, 2008 at 11:09:27PM -0400, Ronald S. Bultje wrote:
> [..]
> >> I also attached a second which removes all useless comments in this
> >> function. These comments are like "// url_split where the line that
> >> the comments refers to in fact, yes really, you wouldn't believe it,
> >> calls url_split. Also many other clueful comments in that func, I
> >> killed them all. I also butchered a line which removes the
> >> authentication from the returned hostname, because that same line is
> >> already in url_split (even if auth is NULL) and rearranges some other
> >> lines to make the function even smaller. (Maybe you want a new thread
> >> for this or have it split for all the re-arrange thingies and
> >> everything?)
> >
> > yes, please split it ...
> 
> Attached, the removing of comments is separated from the fixing up of
> the checks. Since all cosmetic changes affect lines that are changed
> already, I added those in these patches instead of in separate patches
> [1].
> 
> Ronald
> 
> [1]  (I understand that only lines of code unaffected by functional
> changes need to be in a separate cosmetic patch; if lines of code are
> being changed already, I'm allowed to change their cosmetics as well
> in the same patch.)

hmm, you have a strange idea of what spliting in selfcontained patches
means

you start with svn udp.c
and run s# *// PETR:.*##
or something like that
make a patch and submit

Thats the useless comment removing part
the rest is what remains svn di will produce that.
hell i could have done this in the time i wrote this :/


> Index: ffmpeg/libavformat/tcp.c
> ===================================================================
> --- ffmpeg.orig/libavformat/tcp.c	2008-08-20 23:05:37.000000000 -0400
> +++ ffmpeg/libavformat/tcp.c	2008-08-22 17:29:44.000000000 -0400
> @@ -32,19 +32,18 @@
>  static int tcp_open(URLContext *h, const char *uri, int flags)
>  {
>      struct sockaddr_in dest_addr;
> -    char hostname[1024], *q;
[...]
> -    char proto[1024],path[1024],tmp[1024];  // PETR: protocol and path strings
> +    char hostname[1024], proto[1024], path[1024];
>  
>      url_split(proto, sizeof(proto), NULL, 0, hostname, sizeof(hostname),
> -      &port, path, sizeof(path), uri);  // PETR: use url_split
> -    if (strcmp(proto,"tcp")) goto fail; // PETR: check protocol
> -    if ((q = strchr(hostname,'@'))) { strcpy(tmp,q+1); strcpy(hostname,tmp); } // PETR: take only the part after '@' for tcp protocol
> +        &port, path, sizeof(path), uri);
> +    if (strcmp(proto,"tcp")) goto fail;
> +    if ((q = strchr(hostname,'@'))) { strcpy(tmp,q+1); strcpy(hostname,tmp); }

does this compile? i mean you removed q above

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Its not that you shouldnt use gotos but rather that you should write
readable code and code with gotos often but not always is less readable
-------------- 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/20080822/a8297440/attachment.pgp>