Browse Source

remove varargs from logging/quit/in general as much as possible

Kano 12 years ago
parent
commit
7eba963477
8 changed files with 78 additions and 153 deletions
  1. 14 28
      cgminer.c
  2. 5 16
      driver-avalon.c
  3. 4 10
      driver-icarus.c
  4. 0 1
      driver-opencl.c
  5. 14 79
      logging.c
  6. 36 15
      logging.h
  7. 4 3
      miner.h
  8. 1 1
      usbutils.c

+ 14 - 28
cgminer.c

@@ -385,13 +385,16 @@ void get_timestamp(char *f, struct timeval *tv)
 		tm->tm_sec);
 }
 
+static char exit_buf[512];
+
 static void applog_and_exit(const char *fmt, ...)
 {
 	va_list ap;
 
 	va_start(ap, fmt);
-	vapplog(LOG_ERR, fmt, ap);
+	vsnprintf(exit_buf, sizeof(exit_buf), fmt, ap);
 	va_end(ap);
+	_applog(LOG_ERR, exit_buf);
 	exit(1);
 }
 
@@ -2233,24 +2236,16 @@ static void switch_logsize(void)
 }
 
 /* For mandatory printing when mutex is already locked */
-void wlog(const char *f, ...)
+void _wlog(const char *str)
 {
-	va_list ap;
-
-	va_start(ap, f);
-	vw_printw(logwin, f, ap);
-	va_end(ap);
+	wprintw(logwin, "%s", str);
 }
 
 /* Mandatory printing */
-void wlogprint(const char *f, ...)
+void _wlogprint(const char *str)
 {
-	va_list ap;
-
 	if (curses_active_locked()) {
-		va_start(ap, f);
-		vw_printw(logwin, f, ap);
-		va_end(ap);
+		wprintw(logwin, "%s", str);
 		unlock_curses();
 	}
 }
@@ -2261,7 +2256,7 @@ static void switch_logsize(void)
 #endif
 
 #ifdef HAVE_CURSES
-bool log_curses_only(int prio, const char *f, va_list ap)
+bool log_curses_only(int prio, const char *datetime, const char *str)
 {
 	bool high_prio;
 
@@ -2269,7 +2264,7 @@ bool log_curses_only(int prio, const char *f, va_list ap)
 
 	if (curses_active_locked()) {
 		if (!opt_loginput || high_prio) {
-			vw_printw(logwin, f, ap);
+			wprintw(logwin, "%s%s\n", datetime, str);
 			if (high_prio) {
 				touchwin(logwin);
 				wrefresh(logwin);
@@ -4242,7 +4237,7 @@ updated:
 	}
 retry:
 	wlogprint("\nCurrent pool management strategy: %s\n",
-		strategies[pool_strategy]);
+		strategies[pool_strategy].s);
 	if (pool_strategy == POOL_ROTATE)
 		wlogprint("Set to rotate every %d minutes\n", opt_rotate_period);
 	wlogprint("[F]ailover only %s\n", opt_fail_only ? "enabled" : "disabled");
@@ -4313,7 +4308,7 @@ retry:
 		goto updated;
 	} else if (!strncasecmp(&input, "c", 1)) {
 		for (i = 0; i <= TOP_STRATEGY; i++)
-			wlogprint("%d: %s\n", i, strategies[i]);
+			wlogprint("%d: %s\n", i, strategies[i].s);
 		selected = curses_int("Select strategy number type");
 		if (selected < 0 || selected > TOP_STRATEGY) {
 			wlogprint("Invalid selection\n");
@@ -6736,15 +6731,8 @@ static void clean_up(void)
 	curl_global_cleanup();
 }
 
-void quit(int status, const char *format, ...)
+void _quit(int status)
 {
-	if (format) {
-		va_list ap;
-		va_start(ap, format);
-		vapplog(LOG_ERR, format, ap);
-		va_end(ap);
-	}
-
 	clean_up();
 
 #if defined(unix) || defined(__APPLE__)
@@ -7180,12 +7168,10 @@ bool add_cgpu(struct cgpu_info *cgpu)
 struct device_drv *copy_drv(struct device_drv *drv)
 {
 	struct device_drv *copy;
-	char buf[100];
 
 	if (unlikely(!(copy = malloc(sizeof(*copy))))) {
-		sprintf(buf, "Failed to allocate device_drv copy of %s (%s)",
+		quit(1, "Failed to allocate device_drv copy of %s (%s)",
 				drv->name, drv->copy ? "copy" : "original");
-		quit(1, buf);
 	}
 	memcpy(copy, drv, sizeof(*copy));
 	copy->copy = true;

+ 5 - 16
driver-avalon.c

@@ -337,7 +337,6 @@ static int avalon_reset(struct cgpu_info *avalon, bool initial)
 static bool get_options(int this_option_offset, int *baud, int *miner_count,
 			int *asic_count, int *timeout, int *frequency)
 {
-	char err_buf[BUFSIZ+1];
 	char buf[BUFSIZ+1];
 	char *ptr, *comma, *colon, *colon2, *colon3, *colon4;
 	size_t max;
@@ -388,10 +387,8 @@ static bool get_options(int this_option_offset, int *baud, int *miner_count,
 		*baud = 19200;
 		break;
 	default:
-		sprintf(err_buf,
-			"Invalid avalon-options for baud (%s) "
+		quit(1, "Invalid avalon-options for baud (%s) "
 			"must be 115200, 57600, 38400 or 19200", buf);
-		quit(1, err_buf);
 	}
 
 	if (colon && *colon) {
@@ -404,11 +401,9 @@ static bool get_options(int this_option_offset, int *baud, int *miner_count,
 			if (tmp > 0 && tmp <= AVALON_DEFAULT_MINER_NUM) {
 				*miner_count = tmp;
 			} else {
-				sprintf(err_buf,
-					"Invalid avalon-options for "
+				quit(1, "Invalid avalon-options for "
 					"miner_count (%s) must be 1 ~ %d",
 					colon, AVALON_DEFAULT_MINER_NUM);
-				quit(1, err_buf);
 			}
 		}
 
@@ -421,11 +416,9 @@ static bool get_options(int this_option_offset, int *baud, int *miner_count,
 			if (tmp > 0 && tmp <= AVALON_DEFAULT_ASIC_NUM)
 				*asic_count = tmp;
 			else {
-				sprintf(err_buf,
-					"Invalid avalon-options for "
+				quit(1, "Invalid avalon-options for "
 					"asic_count (%s) must be 1 ~ %d",
 					colon2, AVALON_DEFAULT_ASIC_NUM);
-				quit(1, err_buf);
 			}
 
 			if (colon3 && *colon3) {
@@ -437,11 +430,9 @@ static bool get_options(int this_option_offset, int *baud, int *miner_count,
 				if (tmp > 0 && tmp <= 0xff)
 					*timeout = tmp;
 				else {
-					sprintf(err_buf,
-						"Invalid avalon-options for "
+					quit(1, "Invalid avalon-options for "
 						"timeout (%s) must be 1 ~ %d",
 						colon3, 0xff);
-					quit(1, err_buf);
 				}
 				if (colon4 && *colon4) {
 					tmp = atoi(colon4);
@@ -453,10 +444,8 @@ static bool get_options(int this_option_offset, int *baud, int *miner_count,
 						*frequency = tmp;
 						break;
 					default:
-						sprintf(err_buf,
-							"Invalid avalon-options for "
+						quit(1, "Invalid avalon-options for "
 							"frequency must be 256/270/282/300");
-							quit(1, err_buf);
 					}
 				}
 			}

+ 4 - 10
driver-icarus.c

@@ -595,7 +595,6 @@ static void set_timing_mode(int this_option_offset, struct cgpu_info *icarus)
 
 static uint32_t mask(int work_division)
 {
-	char err_buf[BUFSIZ+1];
 	uint32_t nonce_mask = 0x7fffffff;
 
 	// yes we can calculate these, but this way it's easy to see what they are
@@ -613,8 +612,7 @@ static uint32_t mask(int work_division)
 		nonce_mask = 0x1fffffff;
 		break;
 	default:
-		sprintf(err_buf, "Invalid2 icarus-options for work_division (%d) must be 1, 2, 4 or 8", work_division);
-		quit(1, err_buf);
+		quit(1, "Invalid2 icarus-options for work_division (%d) must be 1, 2, 4 or 8", work_division);
 	}
 
 	return nonce_mask;
@@ -622,7 +620,6 @@ static uint32_t mask(int work_division)
 
 static void get_options(int this_option_offset, struct cgpu_info *icarus, int *baud, int *work_division, int *fpga_count)
 {
-	char err_buf[BUFSIZ+1];
 	char buf[BUFSIZ+1];
 	char *ptr, *comma, *colon, *colon2;
 	size_t max;
@@ -691,8 +688,7 @@ static void get_options(int this_option_offset, struct cgpu_info *icarus, int *b
 				*baud = 57600;
 				break;
 			default:
-				sprintf(err_buf, "Invalid icarus-options for baud (%s) must be 115200 or 57600", buf);
-				quit(1, err_buf);
+				quit(1, "Invalid icarus-options for baud (%s) must be 115200 or 57600", buf);
 			}
 		}
 
@@ -707,8 +703,7 @@ static void get_options(int this_option_offset, struct cgpu_info *icarus, int *b
 					*work_division = tmp;
 					*fpga_count = tmp;	// default to the same
 				} else {
-					sprintf(err_buf, "Invalid icarus-options for work_division (%s) must be 1, 2, 4 or 8", colon);
-					quit(1, err_buf);
+					quit(1, "Invalid icarus-options for work_division (%s) must be 1, 2, 4 or 8", colon);
 				}
 			}
 
@@ -717,8 +712,7 @@ static void get_options(int this_option_offset, struct cgpu_info *icarus, int *b
 				if (tmp > 0 && tmp <= *work_division)
 					*fpga_count = tmp;
 				else {
-					sprintf(err_buf, "Invalid icarus-options for fpga_count (%s) must be >0 and <=work_division (%d)", colon2, *work_division);
-					quit(1, err_buf);
+					quit(1, "Invalid icarus-options for fpga_count (%s) must be >0 and <=work_division (%d)", colon2, *work_division);
 				}
 			}
 		}

+ 0 - 1
driver-opencl.c

@@ -53,7 +53,6 @@ extern bool have_opencl;
 extern void *miner_thread(void *userdata);
 extern int dev_from_id(int thr_id);
 extern void tailsprintf(char *f, const char *fmt, ...);
-extern void wlog(const char *f, ...);
 extern void decay_time(double *f, double fadd);
 
 /**********************************************/

+ 14 - 79
logging.c

@@ -1,5 +1,6 @@
 /*
  * Copyright 2011-2012 Con Kolivas
+ * Copyright 2013 Andrew Smith
  *
  * This program 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
@@ -13,7 +14,6 @@
 
 #include "logging.h"
 #include "miner.h"
-// #include "util.h"
 
 bool opt_debug = false;
 bool opt_log_output = false;
@@ -21,66 +21,40 @@ bool opt_log_output = false;
 /* per default priorities higher than LOG_NOTICE are logged */
 int opt_log_level = LOG_NOTICE;
 
-static void my_log_curses(__maybe_unused int prio, char *f, va_list ap)
+static void my_log_curses(int prio, const char *datetime, const char *str)
 {
 	if (opt_quiet && prio != LOG_ERR)
 		return;
 
 #ifdef HAVE_CURSES
 	extern bool use_curses;
-	if (use_curses && log_curses_only(prio, f, ap))
+	if (use_curses && log_curses_only(prio, datetime, str))
 		;
 	else
 #endif
 	{
-		int len = strlen(f);
-
-		strcpy(f + len - 1, "                    \n");
-
 		mutex_lock(&console_lock);
-		vprintf(f, ap);
+		printf("%s%s%s\n", datetime, str, "                    \n");
 		mutex_unlock(&console_lock);
 	}
 }
 
-static void log_generic(int prio, const char *fmt, va_list ap);
-
-void vapplog(int prio, const char *fmt, va_list ap)
-{
-	if (!opt_debug && prio == LOG_DEBUG)
-		return;
-	if (use_syslog || opt_log_output || prio <= LOG_NOTICE)
-		log_generic(prio, fmt, ap);
-}
-
-void _applog(int prio, const char *fmt, ...)
-{
-	va_list ap;
-
-	va_start(ap, fmt);
-	vapplog(prio, fmt, ap);
-	va_end(ap);
-}
-
-
-/* high-level logging functions, based on global opt_log_level */
+/* high-level logging function, based on global opt_log_level */
 
 /*
- * generic log function used by priority specific ones
- * equals vapplog() without additional priority checks
+ * log function
  */
-static void log_generic(int prio, const char *fmt, va_list ap)
+void _applog(int prio, const char *str)
 {
 #ifdef HAVE_SYSLOG_H
 	if (use_syslog) {
-		vsyslog(prio, fmt, ap);
+		syslog(prio, "%s", str);
 	}
 #else
 	if (0) {}
 #endif
 	else {
-		char *f;
-		int len;
+		char datetime[64];
 		struct timeval tv = {0, 0};
 		struct tm *tm;
 
@@ -89,59 +63,20 @@ static void log_generic(int prio, const char *fmt, va_list ap)
 		const time_t tmp_time = tv.tv_sec;
 		tm = localtime(&tmp_time);
 
-		len = 40 + strlen(fmt) + 22;
-		f = alloca(len);
-		sprintf(f, " [%d-%02d-%02d %02d:%02d:%02d] %s\n",
+		sprintf(datetime, " [%d-%02d-%02d %02d:%02d:%02d] ",
 			tm->tm_year + 1900,
 			tm->tm_mon + 1,
 			tm->tm_mday,
 			tm->tm_hour,
 			tm->tm_min,
-			tm->tm_sec,
-			fmt);
+			tm->tm_sec);
+
 		/* Only output to stderr if it's not going to the screen as well */
 		if (!isatty(fileno((FILE *)stderr))) {
-			va_list apc;
-
-			va_copy(apc, ap);
-			vfprintf(stderr, f, apc);	/* atomic write to stderr */
-			va_end(apc);
+			fprintf(stderr, "%s%s\n", datetime, str);	/* atomic write to stderr */
 			fflush(stderr);
 		}
 
-		my_log_curses(prio, f, ap);
-	}
-}
-/* we can not generalize variable argument list */
-#define LOG_TEMPLATE(PRIO)		\
-	if (PRIO <= opt_log_level) {	\
-		va_list ap;		\
-		va_start(ap, fmt);	\
-		vapplog(PRIO, fmt, ap);	\
-		va_end(ap);		\
+		my_log_curses(prio, datetime, str);
 	}
-
-void log_error(const char *fmt, ...)
-{
-	LOG_TEMPLATE(LOG_ERR);
-}
-
-void log_warning(const char *fmt, ...)
-{
-	LOG_TEMPLATE(LOG_WARNING);
-}
-
-void log_notice(const char *fmt, ...)
-{
-	LOG_TEMPLATE(LOG_NOTICE);
-}
-
-void log_info(const char *fmt, ...)
-{
-	LOG_TEMPLATE(LOG_INFO);
-}
-
-void log_debug(const char *fmt, ...)
-{
-	LOG_TEMPLATE(LOG_DEBUG);
 }

+ 36 - 15
logging.h

@@ -17,7 +17,7 @@ enum {
 };
 #endif
 
-/* original / legacy debug flags */
+/* debug flags */
 extern bool opt_debug;
 extern bool opt_log_output;
 extern bool opt_realquiet;
@@ -26,22 +26,43 @@ extern bool want_per_device_stats;
 /* global log_level, messages with lower or equal prio are logged */
 extern int opt_log_level;
 
-/* low-level logging functions with priority parameter */
-extern void vapplog(int prio, const char *fmt, va_list ap);
-extern void _applog(int prio, const char *fmt, ...);
+#define LOGBUFSIZ 256
+
+extern void _applog(int prio, const char *str);
+
 #define applog(prio, fmt, ...) do { \
- char *tmp42; \
- if (0) \
-	sprintf(tmp42, fmt, ##__VA_ARGS__); \
- else \
-	_applog(prio, fmt, ##__VA_ARGS__); \
+	if (opt_debug || prio != LOG_DEBUG) { \
+		if (use_syslog || opt_log_output || prio <= opt_log_level) { \
+			char tmp42[LOGBUFSIZ]; \
+			snprintf(tmp42, sizeof(tmp42), fmt, ##__VA_ARGS__); \
+			_applog(prio, tmp42); \
+		} \
+	} \
+} while (0)
+
+#define quit(status, fmt, ...) do { \
+	if (fmt) { \
+		char tmp42[LOGBUFSIZ]; \
+		snprintf(tmp42, sizeof(tmp42), fmt, ##__VA_ARGS__); \
+		_applog(LOG_ERR, tmp42); \
+	} \
+	_quit(status); \
+} while (0)
+
+#ifdef HAVE_CURSES
+
+#define wlog(fmt, ...) do { \
+	char tmp42[LOGBUFSIZ]; \
+	snprintf(tmp42, sizeof(tmp42), fmt, ##__VA_ARGS__); \
+	_wlog(tmp42); \
 } while (0)
 
-/* high-level logging functions with implicit priority */
-extern void log_error(const char *fmt, ...);
-extern void log_warning(const char *fmt, ...);
-extern void log_notice(const char *fmt, ...);
-extern void log_info(const char *fmt, ...);
-extern void log_debug(const char *fmt, ...);
+#define wlogprint(fmt, ...) do { \
+	char tmp42[LOGBUFSIZ]; \
+	snprintf(tmp42, sizeof(tmp42), fmt, ##__VA_ARGS__); \
+	_wlogprint(tmp42); \
+} while (0)
+
+#endif
 
 #endif /* __LOGGING_H__ */

+ 4 - 3
miner.h

@@ -713,7 +713,7 @@ endian_flip128(void __maybe_unused *dest_p, const void __maybe_unused *src_p)
 }
 #endif
 
-extern void quit(int status, const char *format, ...);
+extern void _quit(int status);
 
 static inline void mutex_lock(pthread_mutex_t *lock)
 {
@@ -1281,7 +1281,8 @@ extern struct work *find_queued_work_bymidstate(struct cgpu_info *cgpu, char *mi
 extern void work_completed(struct cgpu_info *cgpu, struct work *work);
 extern void hash_queued_work(struct thr_info *mythr);
 extern void tailsprintf(char *f, const char *fmt, ...);
-extern void wlogprint(const char *f, ...);
+extern void _wlog(const char *str);
+extern void _wlogprint(const char *str);
 extern int curses_int(const char *query);
 extern char *curses_input(const char *query);
 extern void kill_work(void);
@@ -1292,7 +1293,7 @@ extern void write_config(FILE *fcfg);
 extern void zero_bestshare(void);
 extern void zero_stats(void);
 extern void default_save_file(char *filename);
-extern bool log_curses_only(int prio, const char *f, va_list ap);
+extern bool log_curses_only(int prio, const char *datetime, const char *str);
 extern void clear_logwin(void);
 extern void logwin_update(void);
 extern bool pool_tclear(struct pool *pool, bool *var);

+ 1 - 1
usbutils.c

@@ -2247,7 +2247,7 @@ int _usb_read(struct cgpu_info *cgpu, int ep, char *buf, size_t bufsiz, int *pro
 	USBDEBUG("USB debug: _usb_read(%s (nodev=%s),ep=%d,buf=%p,bufsiz=%zu,proc=%p,timeout=%u,end=%s,cmd=%s,ftdi=%s,readonce=%s)", cgpu->drv->name, bool_str(cgpu->usbinfo.nodev), ep, buf, bufsiz, processed, timeout, end ? (char *)str_text((char *)end) : "NULL", usb_cmdname(cmd), bool_str(ftdi), bool_str(readonce));
 
 	if (bufsiz > USB_MAX_READ)
-		quit(1, "%s USB read request %d too large (max=%d)", cgpu->drv->name, bufsiz, USB_MAX_READ);
+		quit(1, "%s USB read request %zu too large (max=%d)", cgpu->drv->name, bufsiz, USB_MAX_READ);
 
 	if (timeout == DEVTIMEOUT)
 		timeout = usbdev->found->timeout;