[Asterisk-code-review] main: Slight refactor of main. Improve color situation. (asterisk[11])

Matt Jordan asteriskteam at digium.com
Wed Nov 25 22:17:40 CST 2015


Matt Jordan has submitted this change and it was merged.

Change subject: main: Slight refactor of main. Improve color situation.
......................................................................


main: Slight refactor of main. Improve color situation.

Several issues are addressed here:
- main() is large, and half of it is only used if we're not rasterisk;
  fixed by spliting up the daemon part into a separate function.
- Call ast_term_init from rasterisk as well.
- Remove duplicate code reading/writing asterisk history file.
- Attempt to tackle background color issues and color changes that
  occur. Tested by starting asterisk -c until the colors stopped
  changing at odd locations.

ASTERISK-25585 #close

Change-Id: Ib641a0964c59ef9fe6f59efa8ccb481a9580c52f
---
M include/asterisk/term.h
M main/asterisk.c
M main/term.c
3 files changed, 82 insertions(+), 65 deletions(-)

Approvals:
  Kevin Harwell: Looks good to me, but someone else must approve
  Anonymous Coward #1000019: Verified
  Matt Jordan: Looks good to me, approved
  Corey Farrell: Looks good to me, but someone else must approve



diff --git a/include/asterisk/term.h b/include/asterisk/term.h
index bbdc753..82c02be 100644
--- a/include/asterisk/term.h
+++ b/include/asterisk/term.h
@@ -63,7 +63,7 @@
 /*@} */
 
 /*! \brief Maximum number of characters needed for a color escape sequence,
- *         plus a null char */
+ *         and another one for a trailing reset, plus a null char */
 #define AST_TERM_MAX_ESCAPE_CHARS   23
 
 char *term_color(char *outbuf, const char *inbuf, int fgcolor, int bgcolor, int maxout);
diff --git a/main/asterisk.c b/main/asterisk.c
index 45b5b90..901cee1 100644
--- a/main/asterisk.c
+++ b/main/asterisk.c
@@ -243,9 +243,14 @@
 
 char ast_defaultlanguage[MAX_LANGUAGE] = DEFAULT_LANGUAGE;
 
-static int ast_el_add_history(char *);
-static int ast_el_read_history(char *);
-static int ast_el_write_history(char *);
+static int ast_el_add_history(const char *);
+static int ast_el_read_history(const char *);
+static int ast_el_write_history(const char *);
+
+static void ast_el_read_default_histfile(void);
+static void ast_el_write_default_histfile(void);
+
+static void asterisk_daemon(int isroot, const char *runuser, const char *rungroup);
 
 struct _cfg_paths {
 	char config_dir[PATH_MAX];
@@ -1813,13 +1818,7 @@
 	}
 
 	if (ast_opt_console || (ast_opt_remote && !ast_opt_exec)) {
-		char filename[80] = "";
-		if (getenv("HOME")) {
-			snprintf(filename, sizeof(filename), "%s/.asterisk_history", getenv("HOME"));
-		}
-		if (!ast_strlen_zero(filename)) {
-			ast_el_write_history(filename);
-		}
+		ast_el_write_default_histfile();
 		if (consolethread == AST_PTHREADT_NULL || consolethread == pthread_self()) {
 			/* Only end if we are the consolethread, otherwise there's a race with that thread. */
 			if (el != NULL) {
@@ -2058,7 +2057,7 @@
 	}
 }
 
-static int ast_all_zeros(char *s)
+static int ast_all_zeros(const char *s)
 {
 	while (*s) {
 		if (*s > 32)
@@ -2069,7 +2068,7 @@
 }
 
 /* This is the main console CLI command handler.  Run by the main() thread. */
-static void consolehandler(char *s)
+static void consolehandler(const char *s)
 {
 	printf("%s", term_end());
 	fflush(stdout);
@@ -2087,7 +2086,7 @@
 		ast_cli_command(STDOUT_FILENO, s);
 }
 
-static int remoteconsolehandler(char *s)
+static int remoteconsolehandler(const char *s)
 {
 	int ret = 0;
 
@@ -3017,7 +3016,7 @@
 
 #define MAX_HISTORY_COMMAND_LENGTH 256
 
-static int ast_el_add_history(char *buf)
+static int ast_el_add_history(const char *buf)
 {
 	HistEvent ev;
 
@@ -3028,7 +3027,7 @@
 	return (history(el_hist, &ev, H_ENTER, ast_strip(ast_strdupa(buf))));
 }
 
-static int ast_el_write_history(char *filename)
+static int ast_el_write_history(const char *filename)
 {
 	HistEvent ev;
 
@@ -3038,7 +3037,7 @@
 	return (history(el_hist, &ev, H_SAVE, filename));
 }
 
-static int ast_el_read_history(char *filename)
+static int ast_el_read_history(const char *filename)
 {
 	HistEvent ev;
 
@@ -3049,11 +3048,32 @@
 	return history(el_hist, &ev, H_LOAD, filename);
 }
 
+static void ast_el_read_default_histfile(void)
+{
+	char histfile[80] = "";
+	const char *home = getenv("HOME");
+
+	if (!ast_strlen_zero(home)) {
+		snprintf(histfile, sizeof(histfile), "%s/.asterisk_history", home);
+		ast_el_read_history(histfile);
+	}
+}
+
+static void ast_el_write_default_histfile(void)
+{
+	char histfile[80] = "";
+	const char *home = getenv("HOME");
+
+	if (!ast_strlen_zero(home)) {
+		snprintf(histfile, sizeof(histfile), "%s/.asterisk_history", home);
+		ast_el_write_history(histfile);
+	}
+}
+
 static void ast_remotecontrol(char *data)
 {
 	char buf[256] = "";
 	int res;
-	char filename[80] = "";
 	char *hostname;
 	char *cpid;
 	char *version;
@@ -3062,6 +3082,10 @@
 
 	char *ebuf;
 	int num = 0;
+
+	ast_term_init();
+	printf("%s", term_end());
+	fflush(stdout);
 
 	memset(&sig_flags, 0, sizeof(sig_flags));
 	signal(SIGINT, __remote_quit_handler);
@@ -3161,15 +3185,11 @@
 
 	ast_verbose("Connected to Asterisk %s currently running on %s (pid = %d)\n", version, hostname, pid);
 	remotehostname = hostname;
-	if (getenv("HOME"))
-		snprintf(filename, sizeof(filename), "%s/.asterisk_history", getenv("HOME"));
 	if (el_hist == NULL || el == NULL)
 		ast_el_initialize();
+	ast_el_read_default_histfile();
 
 	el_set(el, EL_GETCFN, ast_el_read_char);
-
-	if (!ast_strlen_zero(filename))
-		ast_el_read_history(filename);
 
 	for (;;) {
 		ebuf = (char *)el_gets(el, &num);
@@ -3640,19 +3660,11 @@
 int main(int argc, char *argv[])
 {
 	int c;
-	char filename[80] = "";
-	char hostname[MAXHOSTNAMELEN] = "";
-	char tmp[80];
 	char * xarg = NULL;
 	int x;
-	FILE *f;
-	sigset_t sigs;
-	int num;
 	int isroot = 1, rundir_exists = 0;
-	char *buf;
 	const char *runuser = NULL, *rungroup = NULL;
 	char *remotesock = NULL;
-	int moduleresult;         /*!< Result from the module load subsystem */
 	struct rlimit l;
 
 	/* Remember original args for restart */
@@ -3671,12 +3683,8 @@
 	if (argv[0] && (strstr(argv[0], "rasterisk")) != NULL) {
 		ast_set_flag(&ast_options, AST_OPT_FLAG_NO_FORK | AST_OPT_FLAG_REMOTE);
 	}
-	if (gethostname(hostname, sizeof(hostname)-1))
-		ast_copy_string(hostname, "<Unknown>", sizeof(hostname));
 	ast_mainpid = getpid();
 
-	if (getenv("HOME"))
-		snprintf(filename, sizeof(filename), "%s/.asterisk_history", getenv("HOME"));
 	/* Check for options */
 	while ((c = getopt(argc, argv, "BC:cde:FfG:ghIiL:M:mnpqRrs:TtU:VvWXx:")) != -1) {
 		/*!\note Please keep the ordering here to alphabetical, capital letters
@@ -4017,6 +4025,10 @@
 				quit_handler(0, SHUTDOWN_FAST, 0);
 				exit(0);
 			}
+			ast_term_init();
+			printf("%s", term_end());
+			fflush(stdout);
+
 			print_intro_message(runuser, rungroup);
 			printf("%s", term_quit());
 			ast_remotecontrol(NULL);
@@ -4032,6 +4044,20 @@
 		printf("%s", term_quit());
 		exit(1);
 	}
+
+	/* Not a remote console? Start the daemon. */
+	asterisk_daemon(isroot, runuser, rungroup);
+	return 0;
+}
+
+static void asterisk_daemon(int isroot, const char *runuser, const char *rungroup)
+{
+	FILE *f;
+	sigset_t sigs;
+	int num;
+	char *buf;
+	int moduleresult;         /*!< Result from the module load subsystem */
+	char tmp[80];
 
 	/* This needs to remain as high up in the initial start up as possible.
 	 * daemon causes a fork to occur, which has all sorts of unintended
@@ -4126,9 +4152,7 @@
 	if (ast_opt_console) {
 		if (el_hist == NULL || el == NULL)
 			ast_el_initialize();
-
-		if (!ast_strlen_zero(filename))
-			ast_el_read_history(filename);
+		ast_el_read_default_histfile();
 	}
 
 	ast_ulaw_init();
@@ -4236,7 +4260,7 @@
 
 	/* initialize the data retrieval API */
 	if (ast_data_init()) {
-		printf ("Failed: ast_data_init\n%s", term_quit());
+		printf("Failed: ast_data_init\n%s", term_quit());
 		exit(1);
 	}
 
@@ -4379,6 +4403,11 @@
 		/* Console stuff now... */
 		/* Register our quit function */
 		char title[256];
+		char hostname[MAXHOSTNAMELEN] = "";
+
+		if (gethostname(hostname, sizeof(hostname) - 1)) {
+			ast_copy_string(hostname, "<Unknown>", sizeof(hostname));
+		}
 
 		ast_pthread_create_detached(&mon_sig_flags, NULL, monitor_sig_flags, NULL);
 
@@ -4396,30 +4425,17 @@
 			buf = (char *) el_gets(el, &num);
 
 			if (!buf && write(1, "", 1) < 0)
-				goto lostterm;
+				return; /* quit */
 
 			if (buf) {
 				if (buf[strlen(buf)-1] == '\n')
 					buf[strlen(buf)-1] = '\0';
 
-				consolehandler((char *)buf);
-			} else if (ast_opt_remote && (write(STDOUT_FILENO, "\nUse EXIT or QUIT to exit the asterisk console\n",
-				   strlen("\nUse EXIT or QUIT to exit the asterisk console\n")) < 0)) {
-				/* Whoa, stdout disappeared from under us... Make /dev/null's */
-				int fd;
-				fd = open("/dev/null", O_RDWR);
-				if (fd > -1) {
-					dup2(fd, STDOUT_FILENO);
-					dup2(fd, STDIN_FILENO);
-				} else
-					ast_log(LOG_WARNING, "Failed to open /dev/null to recover from dead console. Bad things will happen!\n");
-				break;
+				consolehandler(buf);
 			}
 		}
 	}
 
+	/* Stall until a quit signal is given */
 	monitor_sig_flags(NULL);
-
-lostterm:
-	return 0;
 }
diff --git a/main/term.c b/main/term.c
index 132b7fa..962ea30 100644
--- a/main/term.c
+++ b/main/term.c
@@ -167,16 +167,14 @@
 		if (ast_opt_light_background) {
 			snprintf(prepdata, sizeof(prepdata), "%c[%dm", ESC, COLOR_BROWN);
 			snprintf(enddata, sizeof(enddata), "%c[%dm", ESC, COLOR_BLACK);
-			snprintf(quitdata, sizeof(quitdata), "%c[0m", ESC);
 		} else if (ast_opt_force_black_background) {
 			snprintf(prepdata, sizeof(prepdata), "%c[%d;%d;%dm", ESC, ATTR_BRIGHT, COLOR_BROWN, COLOR_BLACK + 10);
 			snprintf(enddata, sizeof(enddata), "%c[%d;%d;%dm", ESC, ATTR_RESET, COLOR_WHITE, COLOR_BLACK + 10);
-			snprintf(quitdata, sizeof(quitdata), "%c[0m", ESC);
 		} else {
 			snprintf(prepdata, sizeof(prepdata), "%c[%d;%dm", ESC, ATTR_BRIGHT, COLOR_BROWN);
-			snprintf(enddata, sizeof(enddata), "%c[%d;%dm", ESC, ATTR_RESET, COLOR_WHITE);
-			snprintf(quitdata, sizeof(quitdata), "%c[0m", ESC);
+			snprintf(enddata, sizeof(enddata), "%c[%dm", ESC, ATTR_RESET);
 		}
+		snprintf(quitdata, sizeof(quitdata), "%c[%dm", ESC, ATTR_RESET);
 	}
 	return 0;
 }
@@ -208,9 +206,12 @@
 	}
 
 	if (ast_opt_force_black_background) {
-		snprintf(outbuf, maxout, "%c[%d;%d;%dm%s%c[%d;%dm", ESC, attr, fgcolor, bgcolor + 10, inbuf, ESC, COLOR_WHITE, COLOR_BLACK + 10);
+		if (!bgcolor) {
+			bgcolor = COLOR_BLACK;
+		}
+		snprintf(outbuf, maxout, "%c[%d;%d;%dm%s%s", ESC, attr, fgcolor, bgcolor + 10, inbuf, term_end());
 	} else {
-		snprintf(outbuf, maxout, "%c[%d;%dm%s%c[0m", ESC, attr, fgcolor, inbuf, ESC);
+		snprintf(outbuf, maxout, "%c[%d;%dm%s%s", ESC, attr, fgcolor, inbuf, term_end());
 	}
 	return outbuf;
 }
@@ -234,16 +235,16 @@
 	}
 }
 
-static int check_colors_allowed(int fgcolor)
+static int check_colors_allowed(void)
 {
-	return (!vt100compat || !fgcolor) ? 0 : 1;
+	return vt100compat;
 }
 
 int ast_term_color_code(struct ast_str **str, int fgcolor, int bgcolor)
 {
 	int attr = 0;
 
-	if (!check_colors_allowed(fgcolor)) {
+	if (!check_colors_allowed()) {
 		return -1;
 	}
 
@@ -265,7 +266,7 @@
 {
 	int attr = 0;
 
-	if (!check_colors_allowed(fgcolor)) {
+	if (!check_colors_allowed()) {
 		*outbuf = '\0';
 		return outbuf;
 	}

-- 
To view, visit https://gerrit.asterisk.org/1698
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ib641a0964c59ef9fe6f59efa8ccb481a9580c52f
Gerrit-PatchSet: 6
Gerrit-Project: asterisk
Gerrit-Branch: 11
Gerrit-Owner: Walter Doekes <walter+asterisk at wjd.nu>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>
Gerrit-Reviewer: Walter Doekes <walter+asterisk at wjd.nu>



More information about the asterisk-code-review mailing list