Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:67368
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 74.125.83.52 as permitted sender)
Date: Wed, 8 May 2013 02:26:45 +0200
To: internals@lists.php.net
Message-ID: <20130508002645.GJ28723@methuselah.storkhole.cz>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
User-Agent: Mutt/1.5.21 (2010-09-15)
Subject: [PATCH] faster and safer fpm config reading
From: martin.pelikan@gmail.com (Martin Pelikan)

commit edae18ccf5c51ae0bcc0e9655ead7540fd42dd9f
Author: Martin Pelikan <pelikan@storkhole.cz>
Date:   Wed May 8 02:06:09 2013 +0200

    Faster and safer parsing of fpm configuration.
    
    The current fpm config parser did a read(2) syscall per character,
    which obfuscated debugging strace/ktrace output somewhat.  This also
    makes the 1024 characters per line limit more explicit, so the users
    will know what were they hitting.
    
    It might improve performance during php-fpm load (.03 vs .00 seconds
    on my laptop) but primarily serves to help observing fpm system call
    traces during deployment (in chroot for example).
    
    Signed-off-by: Martin Pelikan <martin.pelikan@gmail.com>

diff --git a/sapi/fpm/fpm/fpm_conf.c b/sapi/fpm/fpm/fpm_conf.c
index 0a8a0e3..e004934 100644
--- a/sapi/fpm/fpm/fpm_conf.c
+++ b/sapi/fpm/fpm/fpm_conf.c
@@ -1447,11 +1447,11 @@ static void fpm_conf_ini_parser(zval *arg1, zval *arg2, zval *arg3, int callback
 
 int fpm_conf_load_ini_file(char *filename TSRMLS_DC) /* {{{ */
 {
+	const int bufsize = 1024;
 	int error = 0;
-	char buf[1024+1];
-	int fd, n;
+	char buf[bufsize], *newl;
+	int fd, pos = 0;
 	int nb_read = 1;
-	char c = '*';
 
 	int ret = 1;
 
@@ -1473,36 +1473,48 @@ int fpm_conf_load_ini_file(char *filename TSRMLS_DC) /* {{{ */
 	}
 
 	ini_lineno = 0;
+	memset(buf, 0, sizeof buf);
 	while (nb_read > 0) {
 		int tmp;
-		memset(buf, 0, sizeof(char) * (1024 + 1));
-		for (n = 0; n < 1024 && (nb_read = read(fd, &c, sizeof(char))) == sizeof(char) && c != '\n'; n++) {
-			buf[n] = c;
-		}
-		buf[n++] = '\n';
-		ini_lineno++;
-		ini_filename = filename;
-		tmp = zend_parse_ini_string(buf, 1, ZEND_INI_SCANNER_NORMAL, (zend_ini_parser_cb_t)fpm_conf_ini_parser, &error TSRMLS_CC);
-		ini_filename = filename;
-		if (error || tmp == FAILURE) {
-			if (ini_include) free(ini_include);
-			ini_recursion--;
-			close(fd);
-			return -1;
-		}
-		if (ini_include) {
-			char *tmp = ini_include;
-			ini_include = NULL;
-			fpm_evaluate_full_path(&tmp, NULL, NULL, 0);
-			fpm_conf_ini_parser_include(tmp, &error TSRMLS_CC);
-			if (error) {
-				free(tmp);
+
+		nb_read = read(fd, buf + pos, sizeof buf - pos);
+		pos = 0;
+		
+		while ((newl = strchr(buf + pos, '\n')) != NULL) {
+			newl[0] = '\0';
+			ini_lineno++;
+			ini_filename = filename;
+			tmp = zend_parse_ini_string(buf + pos, 1, ZEND_INI_SCANNER_NORMAL, (zend_ini_parser_cb_t)fpm_conf_ini_parser, &error TSRMLS_CC);
+			ini_filename = filename;
+			if (error || tmp == FAILURE) {
+				if (ini_include) free(ini_include);
 				ini_recursion--;
 				close(fd);
 				return -1;
 			}
-			free(tmp);
+			if (ini_include) {
+				char *tmp = ini_include;
+				ini_include = NULL;
+				fpm_evaluate_full_path(&tmp, NULL, NULL, 0);
+				fpm_conf_ini_parser_include(tmp, &error TSRMLS_CC);
+				if (error) {
+					free(tmp);
+					ini_recursion--;
+					close(fd);
+					return -1;
+				}
+				free(tmp);
+			}
+
+			pos = newl - buf + 1;
+		}
+		if (nb_read > 0 && pos == 0) {
+			zlog(ZLOG_ERROR, "line %u too long", ini_lineno + 1);
+			close(fd);
+			return -1;
 		}
+		memmove(buf, buf + pos, sizeof buf - pos);
+		pos = sizeof buf - pos;
 	}
 
 	ini_recursion--;