[asterisk-dev] [asterisk-commits] russell: trunk r72701 - in /trunk: ./ include/asterisk/ main/

Luigi Rizzo rizzo at icir.org
Fri Jun 29 15:51:25 CDT 2007


On Fri, Jun 29, 2007 at 08:35:10PM -0000, SVN commits to the Asterisk project wrote:
> Author: russell
> Date: Fri Jun 29 15:35:09 2007
> New Revision: 72701
> 
> URL: http://svn.digium.com/view/asterisk?view=rev&rev=72701
> Log:
> Merge changes from team/russell/http_filetxfer
> 
> Handle transferring large files from the built-in http server.  Previously, the
> code attempted to malloc a block as large as the file itself.  Now it uses the
> sendfile() system call so that the file isn't copied into userspace at all if
> it is available.  Otherwise, it just uses a read/write of small chunks at a time.

Hmmm... not 100% sure, but doesn't this kill HTTPS support, which
requires to go through the fwrite() interface so it can use the
encryption/decryption filter installed by fopencookie()/funopen() ?

Frankly, I believe sendfile() is totally overkill here.
This is asterisk, not a busy web server where you want to optimize
large file transfers.

Surely the use of bounded size buffers is a worthwhile approach,
but i'd suggest to just fix that part and get rid of sendfile.

	cheers
	luigi


> Modified:
>     trunk/configure
>     trunk/configure.ac
>     trunk/include/asterisk/autoconfig.h.in
>     trunk/include/asterisk/http.h
>     trunk/main/http.c
>     trunk/main/manager.c
> 
> Modified: trunk/configure
> URL: http://svn.digium.com/view/asterisk/trunk/configure?view=diff&rev=72701&r1=72700&r2=72701
> ==============================================================================
> --- trunk/configure (original)
> +++ trunk/configure Fri Jun 29 15:35:09 2007
> @@ -1,5 +1,5 @@
>  #! /bin/sh
> -# From configure.ac Revision: 71732 .
> +# From configure.ac Revision: 72539 .
>  # Guess values for system-dependent variables and create Makefiles.
>  # Generated by GNU Autoconf 2.61.
>  #
> @@ -16476,6 +16476,66 @@
>  
>  
>  
> +cat >conftest.$ac_ext <<_ACEOF
> +/* confdefs.h.  */
> +_ACEOF
> +cat confdefs.h >>conftest.$ac_ext
> +cat >>conftest.$ac_ext <<_ACEOF
> +/* end confdefs.h.  */
> +#include <stdlib.h>
> +     #include <sys/sendfile.h>
> +int
> +main ()
> +{
> +sendfile(1, 0, NULL, 1);
> +  ;
> +  return 0;
> +}
> +
> +_ACEOF
> +rm -f conftest.$ac_objext
> +if { (ac_try="$ac_compile"
> +case "(($ac_try" in
> +  *\"* | *\`* | *\\*) ac_try_echo=\$ac_try;;
> +  *) ac_try_echo=$ac_try;;
> +esac
> +eval "echo \"\$as_me:$LINENO: $ac_try_echo\"") >&5
> +  (eval "$ac_compile") 2>conftest.er1
> +  ac_status=$?
> +  grep -v '^ *+' conftest.er1 >conftest.err
> +  rm -f conftest.er1
> +  cat conftest.err >&5
> +  echo "$as_me:$LINENO: \$? = $ac_status" >&5
> +  (exit $ac_status); } && {
> +	 test -z "$ac_c_werror_flag" ||
> +	 test ! -s conftest.err
> +       } && test -s conftest.$ac_objext; then
> +
> +    { echo "$as_me:$LINENO: result: yes" >&5
> +echo "${ECHO_T}yes" >&6; }
> +       have_sendfile=yes
> +
> +else
> +  echo "$as_me: failed program was:" >&5
> +sed 's/^/| /' conftest.$ac_ext >&5
> +
> +
> +    { echo "$as_me:$LINENO: result: no" >&5
> +echo "${ECHO_T}no" >&6; }
> +       have_sendfile=no
> +
> +
> +fi
> +
> +rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
> +if test "${have_sendfile}" = "yes"; then
> +
> +cat >>confdefs.h <<\_ACEOF
> +#define HAVE_SENDFILE 1
> +_ACEOF
> +
> +fi
> +
>  # do the package library checks now
>  
>  
> 
> Modified: trunk/configure.ac
> URL: http://svn.digium.com/view/asterisk/trunk/configure.ac?view=diff&rev=72701&r1=72700&r2=72701
> ==============================================================================
> --- trunk/configure.ac (original)
> +++ trunk/configure.ac Fri Jun 29 15:35:09 2007
> @@ -363,6 +363,23 @@
>                  [AC_DEFINE_UNQUOTED([HAVE_OSX_ATOMICS], 1, [Define to 1 if OSX atomic operations are supported.])])
>  
>  AC_CHECK_SIZEOF(int)
> +
> +AC_COMPILE_IFELSE(
> +  [AC_LANG_PROGRAM(
> +    [#include <stdlib.h>
> +     #include <sys/sendfile.h>],
> +    [sendfile(1, 0, NULL, 1);])
> +  ],[
> +    AC_MSG_RESULT(yes)
> +       have_sendfile=yes
> +  ],[
> +    AC_MSG_RESULT(no)
> +       have_sendfile=no
> +  ]
> +)
> +if test "${have_sendfile}" = "yes"; then
> +  AC_DEFINE([HAVE_SENDFILE], 1, [Define if your system has the sendfile syscall.])
> +fi
>  
>  # do the package library checks now
>  
> 
> Modified: trunk/include/asterisk/autoconfig.h.in
> URL: http://svn.digium.com/view/asterisk/trunk/include/asterisk/autoconfig.h.in?view=diff&rev=72701&r1=72700&r2=72701
> ==============================================================================
> --- trunk/include/asterisk/autoconfig.h.in (original)
> +++ trunk/include/asterisk/autoconfig.h.in Fri Jun 29 15:35:09 2007
> @@ -393,6 +393,9 @@
>  
>  /* Define to 1 if you have the `select' function. */
>  #undef HAVE_SELECT
> +
> +/* Define if your system has the sendfile syscall. */
> +#undef HAVE_SENDFILE
>  
>  /* Define to 1 if you have the `setenv' function. */
>  #undef HAVE_SETENV
> 
> Modified: trunk/include/asterisk/http.h
> URL: http://svn.digium.com/view/asterisk/trunk/include/asterisk/http.h?view=diff&rev=72701&r1=72700&r2=72701
> ==============================================================================
> --- trunk/include/asterisk/http.h (original)
> +++ trunk/include/asterisk/http.h Fri Jun 29 15:35:09 2007
> @@ -147,7 +147,7 @@
>     content is specified) 
>  \endverbatim
>  */
> -typedef struct ast_str *(*ast_http_callback)(struct sockaddr_in *requestor, const char *uri, struct ast_variable *params, int *status, char **title, int *contentlength);
> +typedef struct ast_str *(*ast_http_callback)(struct server_instance *ser, const char *uri, struct ast_variable *params, int *status, char **title, int *contentlength);
>  
>  /*! \brief Definition of a URI reachable in the embedded HTTP server */
>  struct ast_http_uri {
> 
> Modified: trunk/main/http.c
> URL: http://svn.digium.com/view/asterisk/trunk/main/http.c?view=diff&rev=72701&r1=72700&r2=72701
> ==============================================================================
> --- trunk/main/http.c (original)
> +++ trunk/main/http.c Fri Jun 29 15:35:09 2007
> @@ -48,6 +48,10 @@
>  #include <fcntl.h>
>  #include <pthread.h>
>  
> +#ifdef HAVE_SENDFILE
> +#include <sys/sendfile.h>
> +#endif
> +
>  #include "minimime/mm.h"
>  
>  #include "asterisk/cli.h"
> @@ -145,9 +149,8 @@
>  	return wkspace;
>  }
>  
> -static struct ast_str *static_callback(struct sockaddr_in *req, const char *uri, struct ast_variable *vars, int *status, char **title, int *contentlength)
> -{
> -	struct ast_str *result;
> +static struct ast_str *static_callback(struct server_instance *ser, const char *uri, struct ast_variable *vars, int *status, char **title, int *contentlength)
> +{
>  	char *path;
>  	char *ftype;
>  	const char *mtype;
> @@ -155,6 +158,8 @@
>  	struct stat st;
>  	int len;
>  	int fd;
> +	time_t t;
> +	char buf[256];
>  
>  	/* Yuck.  I'm not really sold on this, but if you don't deliver static content it makes your configuration 
>  	   substantially more challenging, but this seems like a rather irritating feature creep on Asterisk. */
> @@ -185,21 +190,28 @@
>  	if (fd < 0)
>  		goto out403;
>  
> -	len = st.st_size + strlen(mtype) + 40;
> -	result = ast_str_create(len);
> -	if (result == NULL)	/* XXX not really but... */
> -		goto out403;
> -
> -	ast_str_append(&result, 0, "Content-type: %s\r\n\r\n", mtype);
> -	*contentlength = read(fd, result->str + result->used, st.st_size);
> -	if (*contentlength < 0) {
> -		close(fd);
> -		ast_free(result);
> -		goto out403;
> -	}
> -	result->used += *contentlength;
> +	time(&t);
> +	strftime(buf, sizeof(buf), "%a, %d %b %Y %H:%M:%S GMT", gmtime(&t));
> +	fprintf(ser->f, "HTTP/1.1 200 OK\r\n"
> +		"Server: Asterisk/%s\r\n"
> +		"Date: %s\r\n"
> +		"Connection: close\r\n"
> +		"Cache-Control: no-cache, no-store\r\n"
> +		"Content-Length: %d\r\n"
> +		"Content-type: %s\r\n\r\n",
> +		ASTERISK_VERSION, buf, (int) st.st_size, mtype);
> +
> +	fflush(ser->f);
> +
> +#ifdef HAVE_SENDFILE
> +	sendfile(ser->fd, fd, NULL, st.st_size);
> +#else
> +	while ((len = read(fd, buf, sizeof(buf))) > 0)
> +		write(ser->fd, buf, len);
> +#endif
> +
>  	close(fd);
> -	return result;
> +	return NULL;
>  
>  out404:
>  	*status = 404;
> @@ -213,7 +225,7 @@
>  }
>  
>  
> -static struct ast_str *httpstatus_callback(struct sockaddr_in *req, const char *uri, struct ast_variable *vars, int *status, char **title, int *contentlength)
> +static struct ast_str *httpstatus_callback(struct server_instance *ser, const char *uri, struct ast_variable *vars, int *status, char **title, int *contentlength)
>  {
>  	struct ast_str *out = ast_str_create(512);
>  	struct ast_variable *v;
> @@ -541,7 +553,7 @@
>  	return ast_http_error(200, "OK", NULL, "File successfully uploaded.");
>  }
>  
> -static struct ast_str *handle_uri(struct sockaddr_in *sin, char *uri, int *status, 
> +static struct ast_str *handle_uri(struct server_instance *ser, char *uri, int *status, 
>  	char **title, int *contentlength, struct ast_variable **cookies, 
>  	unsigned int *static_content)
>  {
> @@ -627,7 +639,7 @@
>  	if (urih) {
>  		if (urih->static_content)
>  			*static_content = 1;
> -		out = urih->callback(sin, uri, vars, status, title, contentlength);
> +		out = urih->callback(ser, uri, vars, status, title, contentlength);
>  		AST_RWLIST_UNLOCK(&uris);
>  	} else {
>  		out = ast_http_error(404, "Not Found", NULL,
> @@ -834,14 +846,12 @@
>  		out = ast_http_error(501, "Not Implemented", NULL,
>  			"Attempt to use unimplemented / unsupported method");
>  	else	/* try to serve it */
> -		out = handle_uri(&ser->requestor, uri, &status, &title, &contentlength, &vars, &static_content);
> +		out = handle_uri(ser, uri, &status, &title, &contentlength, &vars, &static_content);
>  
>  	/* If they aren't mopped up already, clean up the cookies */
>  	if (vars)
>  		ast_variables_destroy(vars);
>  
> -	if (out == NULL)
> -		out = ast_http_error(500, "Internal Error", NULL, "Internal Server Error");
>  	if (out) {
>  		time_t t = time(NULL);
>  		char timebuf[256];
> 
> Modified: trunk/main/manager.c
> URL: http://svn.digium.com/view/asterisk/trunk/main/manager.c?view=diff&rev=72701&r1=72700&r2=72701
> ==============================================================================
> --- trunk/main/manager.c (original)
> +++ trunk/main/manager.c Fri Jun 29 15:35:09 2007
> @@ -3182,19 +3182,19 @@
>  	return out;
>  }
>  
> -static struct ast_str *manager_http_callback(struct sockaddr_in *requestor, const char *uri, struct ast_variable *params, int *status, char **title, int *contentlength)
> -{
> -	return generic_http_callback(FORMAT_HTML, requestor, uri, params, status, title, contentlength);
> -}
> -
> -static struct ast_str *mxml_http_callback(struct sockaddr_in *requestor, const char *uri, struct ast_variable *params, int *status, char **title, int *contentlength)
> -{
> -	return generic_http_callback(FORMAT_XML, requestor, uri, params, status, title, contentlength);
> -}
> -
> -static struct ast_str *rawman_http_callback(struct sockaddr_in *requestor, const char *uri, struct ast_variable *params, int *status, char **title, int *contentlength)
> -{
> -	return generic_http_callback(FORMAT_RAW, requestor, uri, params, status, title, contentlength);
> +static struct ast_str *manager_http_callback(struct server_instance *ser, const char *uri, struct ast_variable *params, int *status, char **title, int *contentlength)
> +{
> +	return generic_http_callback(FORMAT_HTML, &ser->requestor, uri, params, status, title, contentlength);
> +}
> +
> +static struct ast_str *mxml_http_callback(struct server_instance *ser, const char *uri, struct ast_variable *params, int *status, char **title, int *contentlength)
> +{
> +	return generic_http_callback(FORMAT_XML, &ser->requestor, uri, params, status, title, contentlength);
> +}
> +
> +static struct ast_str *rawman_http_callback(struct server_instance *ser, const char *uri, struct ast_variable *params, int *status, char **title, int *contentlength)
> +{
> +	return generic_http_callback(FORMAT_RAW, &ser->requestor, uri, params, status, title, contentlength);
>  }
>  
>  struct ast_http_uri rawmanuri = {
> 
> 
> _______________________________________________
> --Bandwidth and Colocation Provided by http://www.api-digital.com--
> 
> asterisk-commits mailing list
> To UNSUBSCRIBE or update options visit:
>    http://lists.digium.com/mailman/listinfo/asterisk-commits



More information about the asterisk-dev mailing list