[MPlayer-dev-eng] [PATCH] Add mp_strings.c with mp_asprintf function.

Clément Bœsch ubitux at gmail.com
Thu Feb 24 01:09:28 CET 2011


On Sat, Feb 19, 2011 at 04:40:31PM +0100, Reimar Döffinger wrote:
> On Sat, Feb 19, 2011 at 02:53:27PM +0100, Clément Bœsch wrote:
> > @@ -164,14 +164,21 @@ check4proxies( URL_t *url ) {
> >  #endif
> >  
> >  			mp_msg(MSGT_NETWORK,MSGL_V,"Using HTTP proxy: %s\n", proxy_url->url );
> > -			len = make_http_proxy_url(proxy_url, url->url, NULL, 0) + 1;
> > -			new_url = malloc(len);
> > +
> > +			if (proxy_url->username)
> > +				mp_asprintf(&new_url, "http_proxy://%s:%s@%s:%d/%s",
> > +						proxy_url->username,
> > +						proxy_url->password ? proxy_url->password : "",
> > +						proxy_url->hostname, proxy_url->port, url->url);
> > +			else
> > +				mp_asprintf(&new_url, "http_proxy://%s:%d/%s",
> > +						proxy_url->hostname, proxy_url->port, url->url);
> > +
> >  			if( new_url==NULL ) {
> >  				mp_msg(MSGT_NETWORK,MSGL_FATAL,MSGTR_MemAllocFailed);
> >  				url_free(proxy_url);
> >  				return url_out;
> >  			}
> > -			make_http_proxy_url(proxy_url, url->url, new_url, len);
> >  			tmp_url = url_new( new_url );
> 
> Please keep the separate functions, it makes the code easier to read.
> 

Ok, changed.

> > -      snprintf(meta, sizeof(meta), "%.4s", (char *) &sh_video->format);
> > +      mp_asprintf(&meta, "%.4s", (char *) &sh_video->format);
> >      else
> > -      snprintf(meta, sizeof(meta), "0x%08X", sh_video->format);
> > -    return strdup(meta);
> > +      mp_asprintf(&meta, "0x%08X", sh_video->format);
> > +    return meta;
> 
> That would be a good bit nicer if the function just returned
> the pointer...

Sure, it was just to be consistent with GNU asprintf.

> Is there are reasonable chance we will ever want/need the int
> return value?

I don't think so.

Patches updated.

-- 
Clément B.
-------------- next part --------------
From d760983687bfa5f201dbde6c247f3c1f01806c39 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cl=C3=A9ment=20B=C5=93sch?= <ubitux at gmail.com>
Date: Wed, 12 Jan 2011 23:37:11 +0100
Subject: [PATCH 1/3] Add mp_strings.c with mp_asprintf function.

---
 Makefile     |    1 +
 mp_strings.c |   50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 mp_strings.h |   26 ++++++++++++++++++++++++++
 3 files changed, 77 insertions(+), 0 deletions(-)
 create mode 100644 mp_strings.c
 create mode 100644 mp_strings.h

diff --git a/Makefile b/Makefile
index 5e5ef6d..2996392 100644
--- a/Makefile
+++ b/Makefile
@@ -301,6 +301,7 @@ SRCS_COMMON = asxparser.c \
               m_option.c \
               m_struct.c \
               mp_msg.c \
+              mp_strings.c \
               mpcommon.c \
               parser-cfg.c \
               path.c \
diff --git a/mp_strings.c b/mp_strings.c
new file mode 100644
index 0000000..4558101
--- /dev/null
+++ b/mp_strings.c
@@ -0,0 +1,50 @@
+/*
+ * Strings utilities
+ *
+ * This file is part of MPlayer.
+ *
+ * MPlayer is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * MPlayer is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with MPlayer; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include <stdlib.h>
+#include <stdarg.h>
+#include <stdio.h>
+#include "mp_strings.h"
+
+char *mp_asprintf(const char *fmt, ...)
+{
+    char *p = NULL;
+    va_list va, va_bak;
+    int len;
+
+    va_start(va, fmt);
+    va_copy(va_bak, va);
+
+    len = vsnprintf(NULL, 0, fmt, va);
+    if (len < 0)
+        goto end;
+
+    p = malloc(len + 1);
+    if (!p)
+        goto end;
+
+    len = vsnprintf(p, len + 1, fmt, va_bak);
+    if (len < 0)
+        free(p), p = NULL;
+
+end:
+    va_end(va);
+    return p;
+}
diff --git a/mp_strings.h b/mp_strings.h
new file mode 100644
index 0000000..8ad101f
--- /dev/null
+++ b/mp_strings.h
@@ -0,0 +1,26 @@
+/*
+ * Strings utilities
+ *
+ * This file is part of MPlayer.
+ *
+ * MPlayer is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * MPlayer is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with MPlayer; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#ifndef MPLAYER_MP_STRINGS_H
+#define MPLAYER_MP_STRINGS_H
+
+char *mp_asprintf(const char *fmt, ...);
+
+#endif /* !MPLAYER_MP_STRINGS_H */
-- 
1.7.4.1

-------------- next part --------------
From d61ea54a3848c5027270d7a456dbf72b329812db Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cl=C3=A9ment=20B=C5=93sch?= <ubitux at gmail.com>
Date: Sun, 13 Feb 2011 12:16:01 +0100
Subject: [PATCH 2/3] Use mp_asprintf in make_noauth_url and make_http_proxy_url.

---
 stream/network.c |    5 +----
 stream/url.c     |   35 +++++++++++++++--------------------
 stream/url.h     |    2 +-
 3 files changed, 17 insertions(+), 25 deletions(-)

diff --git a/stream/network.c b/stream/network.c
index 1f1d382..fcb58d7 100644
--- a/stream/network.c
+++ b/stream/network.c
@@ -143,7 +143,6 @@ check4proxies( URL_t *url ) {
 		proxy = getenv("http_proxy");
 		if( proxy!=NULL ) {
 			// We got a proxy, build the URL to use it
-			int len;
 			char *new_url;
 			URL_t *tmp_url;
 			URL_t *proxy_url = url_new( proxy );
@@ -164,14 +163,12 @@ check4proxies( URL_t *url ) {
 #endif
 
 			mp_msg(MSGT_NETWORK,MSGL_V,"Using HTTP proxy: %s\n", proxy_url->url );
-			len = make_http_proxy_url(proxy_url, url->url, NULL, 0) + 1;
-			new_url = malloc(len);
+			new_url = get_http_proxy_url(proxy_url, url->url);
 			if( new_url==NULL ) {
 				mp_msg(MSGT_NETWORK,MSGL_FATAL,MSGTR_MemAllocFailed);
 				url_free(proxy_url);
 				return url_out;
 			}
-			make_http_proxy_url(proxy_url, url->url, new_url, len);
 			tmp_url = url_new( new_url );
 			if( tmp_url==NULL ) {
 				free( new_url );
diff --git a/stream/url.c b/stream/url.c
index af530f0..ca7d1e1 100644
--- a/stream/url.c
+++ b/stream/url.c
@@ -28,6 +28,7 @@
 
 #include "url.h"
 #include "mp_msg.h"
+#include "mp_strings.h"
 #include "help_mp.h"
 
 #ifndef SIZE_MAX
@@ -58,32 +59,31 @@ URL_t *url_redirect(URL_t **url, const char *redir) {
   return res;
 }
 
-static int make_noauth_url(URL_t *url, char *dst, int dst_size)
+static char *get_noauth_url(URL_t *url)
 {
     if (url->port)
-        return snprintf(dst, dst_size, "%s://%s:%d%s", url->protocol,
-                        url->hostname, url->port, url->file);
+        return mp_asprintf("%s://%s:%d%s",
+                           url->protocol, url->hostname, url->port, url->file);
     else
-        return snprintf(dst, dst_size, "%s://%s%s", url->protocol,
-                        url->hostname, url->file);
+        return mp_asprintf("%s://%s%s",
+                           url->protocol, url->hostname, url->file);
 }
 
-int make_http_proxy_url(URL_t *proxy, const char *host_url, char *dst,
-                        int dst_size)
+char *get_http_proxy_url(URL_t *proxy, const char *host_url)
 {
     if (proxy->username)
-        return snprintf(dst, dst_size, "http_proxy://%s:%s@%s:%d/%s",
-                        proxy->username,
-                        proxy->password ? proxy->password : "",
-                        proxy->hostname, proxy->port, host_url);
+        return mp_asprintf("http_proxy://%s:%s@%s:%d/%s",
+                           proxy->username,
+                           proxy->password ? proxy->password : "",
+                           proxy->hostname, proxy->port, host_url);
     else
-        return snprintf(dst, dst_size, "http_proxy://%s:%d/%s",
-                        proxy->hostname, proxy->port, host_url);
+        return mp_asprintf("http_proxy://%s:%d/%s",
+                           proxy->hostname, proxy->port, host_url);
 }
 
 URL_t*
 url_new(const char* url) {
-	int pos1, pos2,v6addr = 0, noauth_len;
+	int pos1, pos2,v6addr = 0;
 	URL_t* Curl = NULL;
         char *escfilename=NULL;
 	char *ptr1=NULL, *ptr2=NULL, *ptr3=NULL, *ptr4=NULL;
@@ -252,16 +252,11 @@ url_new(const char* url) {
 		strcpy(Curl->file, "/");
 	}
 
-	noauth_len = make_noauth_url(Curl, NULL, 0);
-	if (noauth_len > 0) {
-		noauth_len++;
-		Curl->noauth_url = malloc(noauth_len);
+	Curl->noauth_url = get_noauth_url(Curl);
 		if (!Curl->noauth_url) {
 			mp_msg(MSGT_NETWORK, MSGL_FATAL, MSGTR_MemAllocFailed);
 			goto err_out;
 		}
-		make_noauth_url(Curl, Curl->noauth_url, noauth_len);
-	}
 
         free(escfilename);
 	return Curl;
diff --git a/stream/url.h b/stream/url.h
index b75978a..f5c75fc 100644
--- a/stream/url.h
+++ b/stream/url.h
@@ -38,7 +38,7 @@ typedef struct {
 
 URL_t *url_redirect(URL_t **url, const char *redir);
 
-int make_http_proxy_url(URL_t *proxy, const char *host_url, char *dst, int dst_size);
+char *get_http_proxy_url(URL_t *proxy, const char *host_url);
 
 URL_t* url_new(const char* url);
 void   url_free(URL_t* url);
-- 
1.7.4.1

-------------- next part --------------
From d9e6d4201ce207cc52fe7b87b75dcf8997a2f830 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cl=C3=A9ment=20B=C5=93sch?= <ubitux at gmail.com>
Date: Sun, 13 Feb 2011 12:24:48 +0100
Subject: [PATCH 3/3] Use mp_asprintf in get_metadata instead of limited stack buffer.

---
 mplayer.c |   19 ++++++-------------
 1 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/mplayer.c b/mplayer.c
index 28eb700..5034b98 100644
--- a/mplayer.c
+++ b/mplayer.c
@@ -438,7 +438,6 @@ static char *get_demuxer_info (char *tag) {
 }
 
 char *get_metadata (metadata_t type) {
-  char meta[128];
   sh_audio_t * const sh_audio = mpctx->sh_audio;
   sh_video_t * const sh_video = mpctx->sh_video;
 
@@ -460,18 +459,14 @@ char *get_metadata (metadata_t type) {
     else if (sh_video->format == 0x10000005)
       return strdup("h264");
     else if (sh_video->format >= 0x20202020)
-      snprintf(meta, sizeof(meta), "%.4s", (char *) &sh_video->format);
-    else
-      snprintf(meta, sizeof(meta), "0x%08X", sh_video->format);
-    return strdup(meta);
+      return mp_asprintf("%.4s", (char *)&sh_video->format);
+    return mp_asprintf("0x%08X", sh_video->format);
 
   case META_VIDEO_BITRATE:
-    snprintf(meta, sizeof(meta), "%d kbps", (int) (sh_video->i_bps * 8 / 1024));
-    return strdup(meta);
+    return mp_asprintf("%d kbps", (int)(sh_video->i_bps * 8 / 1024));
 
   case META_VIDEO_RESOLUTION:
-    snprintf(meta, sizeof(meta), "%d x %d", sh_video->disp_w, sh_video->disp_h);
-    return strdup(meta);
+    return mp_asprintf("%d x %d", sh_video->disp_w, sh_video->disp_h);
 
   case META_AUDIO_CODEC:
     if (sh_audio->codec && sh_audio->codec->name)
@@ -479,12 +474,10 @@ char *get_metadata (metadata_t type) {
     break;
 
   case META_AUDIO_BITRATE:
-    snprintf(meta, sizeof(meta), "%d kbps", (int)(sh_audio->i_bps * 8 / 1000));
-    return strdup(meta);
+    return mp_asprintf("%d kbps", (int)(sh_audio->i_bps * 8 / 1000));
 
   case META_AUDIO_SAMPLES:
-    snprintf(meta, sizeof(meta), "%d Hz, %d ch.", sh_audio->samplerate, sh_audio->channels);
-    return strdup(meta);
+    return mp_asprintf("%d Hz, %d ch.", sh_audio->samplerate, sh_audio->channels);
 
   /* check for valid demuxer */
   case META_INFO_TITLE:
-- 
1.7.4.1

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20110224/b684db1d/attachment.pgp>


More information about the MPlayer-dev-eng mailing list