Suppress more warning and add a lint target

Makefile.am

@@ -20,11 +20,15 @@ child$(EXEEXT): $(child_SOURCES:.s=.o)
 child_bin.o: child$(EXEEXT)
 	$(LD) -r -b binary -o $@ $<
 
+ALL_CHECKS=
+LINT=
 SUBDIRS = 
 if CHECK
 noinst_LIBRARIES = libcheck_asmsh.a
 libcheck_asmsh_a_LDDADD=child_bin.o
 
+ALL_CHECKS += check
+
 if HAVE_GCOV
 libcheck_asmsh_a_CFLAGS = @CFLAGS@ -fprofile-arcs -ftest-coverage
 
@@ -55,9 +59,20 @@ endif
 
 SUBDIRS += tests
 
-checks: check coverage
+
+ALL_CHECKS += coverage
 
 coverage: $(LCOV_HTML)
 
 endif
 
+if HAVE_CPPCHECK
+LINT += cppcheck
+cppcheck:
+	$(CPPCHECK) --enable=all --inline-suppr --check-config --suppress=missingIncludeSystem -q . -I .
+endif
+
+checks: $(ALL_CHECKS)
+
+lint: $(LINT)
+

asm_env.h

@@ -50,18 +50,18 @@ struct asmsh_env_s
 	 * perm allowing use to write code to be executed by the
 	 * child
 	 */
-	void *txt_map_addr;
+	unsigned char *txt_map_addr;
 	/** child's map size */
 	size_t txt_map_sz;
 	/** Pointer on current write addr in child's map */
-	void *txt_map_ptr;
+	unsigned char *txt_map_ptr;
 
 	/** Pointer on the next addr where we should write some
 	 *  compiled bytecode */
-	void *code_write_ptr;
+	unsigned char *code_write_ptr;
 
 	/** Pointer, in child memory, of the stack */
-	void *stack_addr;
+	unsigned char *stack_addr;
 	size_t stack_sz;
 
 };

asmsh.c

@@ -31,7 +31,6 @@ int main(int argc, char *argv[], char *envp[])
 	asmsh_t sh;
 	
 	int ret = 0;
-	char *cmd;
 	char prompt[64];
 	bzero(prompt, sizeof(prompt));
 
@@ -54,6 +53,7 @@ int main(int argc, char *argv[], char *envp[])
 
 	while(1)
 	{
+		char *cmd;
 		snprintf(prompt, sizeof(prompt)-1,
 				"asmsh@%p > ",
 				(void*)sh.env->regs.rip);
@@ -83,7 +83,7 @@ int main(int argc, char *argv[], char *envp[])
 	HIST_ENTRY **hist = history_list();
 	int hist_count = 0;
 	for(HIST_ENTRY **p=hist; *p;p++){ hist_count++; }
-	const char **hstr = alloca((hist_count+1)*sizeof(char*));
+	const char *hstr[(hist_count+1)*sizeof(char*)];
 	for(int i=0; i<hist_count; i++){ hstr[i] = hist[i]->line; }
 	hstr[hist_count]=NULL;
 

compile.c

@@ -175,12 +175,18 @@ int asmsh_asmc_compile_unsafe(asmsh_asmc_ctx_t *ctx, const char *_instr,
 	sigset_t sigfdmask, oldmask;
 	fd_set sigset;
 	struct timeval stimeout;
-	char *instr;
 
 	sigemptyset(&sigfdmask);
 	sigaddset(&sigfdmask, SIGCHLD);
 	sigfillset(&sigfdmask);
 
+	instr_sz = strlen(_instr);
+	char instr[instr_sz+1];
+	memcpy(instr, _instr, instr_sz);
+	instr[instr_sz] = '\n';
+	instr[instr_sz+1] = '\0';
+	instr_sz++;
+
 	/// TODO : early (context ?) sigprocmask ?
 	if((sigprocmask(SIG_BLOCK, &sigfdmask, &oldmask) == -1))
 	{
@@ -196,13 +202,6 @@ int asmsh_asmc_compile_unsafe(asmsh_asmc_ctx_t *ctx, const char *_instr,
 	}
 
 
-	instr_sz = strlen(_instr);
-	instr = alloca(instr_sz+1);
-	memcpy(instr, _instr, instr_sz);
-	instr[instr_sz] = '\n';
-	instr[instr_sz+1] = '\0';
-	instr_sz++;
-
 	ret = write(ctx->child.pipe_stdin, instr, instr_sz);
 	if(ret < 0)
 	{
@@ -302,7 +301,7 @@ err_stderr:
 			else if (readed > 0)
 			{
 				curbuf[readed] = '\0';
-				inbuf += readed;
+				//inbuf += readed;
 				curbuf=buf;
 				for(char *tbuf=buf;  *tbuf; tbuf++)
 				{

completion.c

@@ -152,7 +152,7 @@ char **asmsh_compl_arg(const char *buf, const char *text, int start)
 
 	tlen = text?strlen(text):0;
 
-	for(int i=0; i<(sizeof(x86_64_instr)/sizeof(*x86_64_instr))-1; i++)
+	for(i=0; i<(sizeof(x86_64_instr)/sizeof(*x86_64_instr))-1; i++)
 	{
 		int cmp;
 		const char *instr = x86_64_instr[i].mnemo;
@@ -259,9 +259,9 @@ char **asmsh_compl_arg(const char *buf, const char *text, int start)
 			{
 				//full match on arg0 adding ","
 				char *tmp = ret[i];
-				char buf[32];
+				char _buf[32];
-				snprintf(buf, 31, "%s,", tmp);
+				snprintf(_buf, 31, "%s,", tmp);
-				ret[i] = strdup(buf);
+				ret[i] = strdup(_buf);
 			}
 			i++;
 		}

configure.ac

@@ -55,9 +55,11 @@ then
 		fi
 	fi
 fi
+AC_CHECK_PROGS([CPPCHECK], [cppcheck])
 
 AM_CONDITIONAL([HAVE_GCOV], [test -n "$GCOV"])
 AM_CONDITIONAL([HAVE_LCOV], [test -n "$LCOV"])
+AM_CONDITIONAL([HAVE_CPPCHECK], [test -n "$CPPCHECK"])
 
 # Checks for libraries.
 AC_CHECK_LIB(readline, readline)

history.c

@@ -3,7 +3,6 @@
 char * history_filename_init(const char *homedir)
 {
 	const char fmt[] = "%s/.local/share/asmsh";
-	char *path;
 	int sz;
 	int fd = open("/dev/null", O_WRONLY);
 	if(!fd)
@@ -24,7 +23,7 @@ char * history_filename_init(const char *homedir)
 		return NULL;
 	}
 	
-	path = alloca(sz+1);
+	char path[sz+1];
 	sprintf(path, fmt, home);
 	path[sz]='\0';
 	if(access(path, F_OK) < 0)

logger.c

@@ -73,7 +73,7 @@ int asmsh_logger_dprint_fmt(int fd, asmsh_logger_t *logger, asmsh_log_fmt_f *cus
 {
 	const int BUF_ALLOC = 4096;
 	char *buf;
-	int bufsz, ret, res;
+	int bufsz, res;
 	asmsh_log_msg_t *cur;
 	asmsh_log_fmt_f *fmt;
 
@@ -89,7 +89,7 @@ int asmsh_logger_dprint_fmt(int fd, asmsh_logger_t *logger, asmsh_log_fmt_f *cus
 	res = 0;
 	while(cur != logger->nxt)
 	{
-		ret = fmt(cur, buf, bufsz);
+		int ret = fmt(cur, buf, bufsz);
 		if(ret < 0)
 		{
 			perror("Unable to format log message");
@@ -117,7 +117,6 @@ int asmsh_logger_strdup(asmsh_logger_t *logger, char **result, size_t *res_sz)
 	size_t buf_sz;
 	asmsh_log_msg_t *cur;
 	char *buf_ptr;
-	int ret;
 	int err;
 
 	*res_sz = 0;
@@ -133,7 +132,7 @@ int asmsh_logger_strdup(asmsh_logger_t *logger, char **result, size_t *res_sz)
 	cur = (asmsh_log_msg_t*)logger->msgs;
 	while(cur != logger->nxt)
 	{
-		ret = logger->fmt(cur, buf_ptr, buf_sz - *res_sz);
+		int ret = logger->fmt(cur, buf_ptr, buf_sz - *res_sz);
 		if(ret == (*res_sz - buf_sz))
 		{
 			buf_sz += BUF_ALLOC;
@@ -173,10 +172,10 @@ int asmsh_log(asmsh_logger_t *logger, asmsh_loglevel_t lvl, const char caller[],
 
 	if(logger->msgs_alloc <= logger->msgs_sz + total_len)
 	{
-		void *tmp;
+		asmsh_log_msg_t *tmp;
 		logger->msgs_alloc += ASMSH_LOG_BUFFER_ALLOC;
 		tmp = realloc(logger->msgs, logger->msgs_alloc);
-		if(tmp <= 0)
+		if(tmp == NULL)
 		{
 			return -1;
 		}
@@ -208,7 +207,7 @@ int asmsh_log(asmsh_logger_t *logger, asmsh_loglevel_t lvl, const char caller[],
 	strncpy(logger->nxt->caller, caller, caller_len+1);
 	strncpy(logger->nxt->msg, msg, msg_len+1);
 
-	logger->nxt->nxt = (asmsh_log_msg_t*)((void*)logger->nxt->msg+total_len);
+	logger->nxt->nxt = (asmsh_log_msg_t*)((char*)logger->nxt->msg+total_len);
 	logger->nxt = logger->nxt->nxt;
 
 	return 0;

mmap_parse.h

@@ -37,8 +37,8 @@ typedef struct child_mmap_list child_mmap_l;
 
 struct child_mmap_s
 {
-	void *start;
+	char *start;
-	void *stop;
+	char *stop;
 	int perm;
 	size_t offset;
 	dev_t device;

shell_cmds.c

@@ -52,7 +52,8 @@ asmsh_cmd_args_t *asmsh_cmd_parse(const char *cmd)
 	int cmdlen, err, argno;
 	const char *ptr, *arg;
 
-	if(!cmd || !*cmd || *cmd != '.') { return NULL; }
+	if(!cmd || !*cmd) { return NULL; }
+	if (*cmd != '.') { return NULL; }
 
 	if(!(ret = malloc(sizeof(*ret))))
 	{
@@ -137,6 +138,7 @@ int asmsh_cmd_bcode(asmsh_t *sh, char *argbuf, int bufsz, int argc, char **args)
 {
 	asmsh_bytecode_t bcode;
 	char _buf[256], *buf;
+	char code[256];
 	const char *instr;
 	int ret;
 
@@ -163,7 +165,6 @@ int asmsh_cmd_bcode(asmsh_t *sh, char *argbuf, int bufsz, int argc, char **args)
 	else
 	{
 		char **arg;
-		char code[256];
 		int sz = 0;
 		for(arg=args;*arg;arg++)
 		{

tests/regen.sh

@@ -24,13 +24,13 @@ clean-local:
 	-rm -rf *.gcov *.gcda *.gcno
 
 \$(CHILD):
-	make -C \$(top_builddir) \$(CHILD_NAME)
+	\$(MAKE) -C \$(top_builddir) \$(CHILD_NAME)
 
 \$(TESTLIB):
-	make -C \$(top_builddir) \$(TESTLIB_NAME)
+	\$(MAKE) -C \$(top_builddir) \$(TESTLIB_NAME)
 
 \$(CHILD_OBJ):
-	make -C \$(top_builddir) \$(CHILD_OBJNAME)
+	\$(MAKE) -C \$(top_builddir) \$(CHILD_OBJNAME)
 
 .PHONY: \$(TESTLIB) \$(CHILD)
 __EOF__

tests/tests_history.c

@@ -0,0 +1,76 @@
+#include "config.h"
+#include <check.h>
+#include <errno.h>
+#include <stdio.h>
+#include <string.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+#include "asmsh_check.h"
+#include "history.h"
+
+START_TEST(test_filename)
+{
+	char *fname;
+	char expt_path[4096];
+
+	fname = history_filename_init("/foo");
+	ck_assert_ptr_nonnull(fname);
+
+	snprintf(expt_path, sizeof(expt_path), "/foo/.local/share/asmsh/%s",
+			ASMSH_HISTORY_FILE);
+	ck_assert_str_eq(expt_path, fname);
+
+	fname = history_filename_init(NULL);
+	if(getenv("HOME"))
+	{
+		snprintf(expt_path, sizeof(expt_path), "%s/.local/share/asmsh/%s",
+				getenv("HOME"),
+				ASMSH_HISTORY_FILE);
+		ck_assert_ptr_nonnull(fname);
+		ck_assert_str_eq(expt_path, fname);
+	}
+	else
+	{
+		ck_assert_ptr_null(fname);
+	}
+	
+}
+END_TEST
+
+START_TEST(test_save)
+{
+	// Do not test the written content, only check for segfaults
+	const char * const hist[]={"foo", "bar", "super history"};
+	save_history("/dev/null", hist);
+
+}
+END_TEST
+
+#define SAMPLE_HISTORY "samples/sample.history"
+
+static const char * const samples[] = {
+	"hello world!",
+	"mov $1312, %rax",
+	"syscall",
+};
+static int mock_add_calls = 0;
+
+static void mock_add_history(const char *str)
+{
+	int sz = sizeof(samples)/sizeof(*samples);
+	ck_assert(mock_add_calls < sz);
+	ck_assert_str_eq(samples[mock_add_calls], str);
+	mock_add_calls++;
+}
+
+START_TEST(test_load)
+{
+	load_history(SAMPLE_HISTORY, mock_add_history);
+}
+
+ASMSH_CHECK_START("history utils", "testing history store/load functions")
+	ASMSH_ADD_TEST(test_filename);
+	ASMSH_ADD_TEST(test_save);
+	ASMSH_ADD_TEST(test_load);
+ASMSH_CHECK_END

tests/tests_logger.c

@@ -10,12 +10,11 @@
 
 START_TEST(test_logger_new)
 {
-	asmsh_logger_t *logger;
 	asmsh_loglevel_t lvls[] = {ASMSH_DEBUG, ASMSH_WARN, ASMSH_FATAL};
 
 	for(int i = 0; i<sizeof(lvls) / sizeof(*lvls); i++)
 	{
-		logger = asmsh_logger_new(lvls[i]);
+		asmsh_logger_t *logger = asmsh_logger_new(lvls[i]);
 
 		ck_assert_ptr_nonnull(logger);
 		ck_assert_int_eq((logger->min_level), lvls[i]);
@@ -63,7 +62,6 @@ END_TEST
 START_TEST(test_log_function_lvl_nolog)
 {
 	asmsh_logger_t *logger;
-	asmsh_log_msg_t *msg;
 
 	logger = asmsh_logger_new(ASMSH_INFO);
 
@@ -172,7 +170,6 @@ START_TEST(test_log_dprint)
 
 	const char *caller, *testmsg;
 	char *exptr, *expt, *res;
-	size_t total_len = 0;
 	int tmp_fd;
 	const char _tmpname[] = "/tmp/dprintXXXXXX";
 	char *tmpname = strdupa(_tmpname);
@@ -182,8 +179,6 @@ START_TEST(test_log_dprint)
 		
 		caller = msg_samples[i][0];
 		testmsg = msg_samples[i][1];
-		total_len += sizeof(asmsh_log_msg_t);
-		total_len += strlen(caller) + strlen(testmsg) + 2;
 
 		ck_assert_int_eq(asmsh_log(logger, ASMSH_INFO, caller, testmsg), 0);
 	}

tests/tests_mmap.c

@@ -57,22 +57,22 @@ START_TEST (test_parse_fd)
 		 PROT_READ | PROT_EXEC | MAP_PRIVATE, 0, (8<<8)+2, 173521,
 		 "/usr/bin/dbus-daemon"},
 		{(void*)0x651000, (void*)0x652000,
-		 PROT_READ | MAP_PRIVATE, 0, (8<<8)+2, 173521,
+		 PROT_READ | MAP_PRIVATE, 0x51000, (8<<8)+2, 173521,
 		 "/usr/bin/dbus-daemon"},
 		{(void*)0x652000, (void*)0x655000,
-		 PROT_READ | PROT_WRITE | MAP_PRIVATE, 0, (8<<8)+2, 173521,
+		 PROT_READ | PROT_WRITE | MAP_PRIVATE, 0x52000, (8<<8)+2, 173521,
 		 "/usr/bin/dbus-daemon"},
-		{(void*)0xe3000, (void*)0xe24000,
+		{(void*)0xe03000, (void*)0xe24000,
 		 PROT_READ | PROT_WRITE | MAP_PRIVATE, 0, 0, 0,
 		 "[heap]"},
-		{(void*)0x24000, (void*)0x11f7000,
+		{(void*)0xe24000, (void*)0x11f7000,
 		 PROT_READ | PROT_WRITE | MAP_SHARED, 0, 0, 0,
 		 "[heap]"},
 		{(void*)0x35b1a20000, (void*)0x35b1a21000,
 		 PROT_READ | PROT_WRITE | MAP_PRIVATE, 0x20000, (13<<8)+12, 135522,
 		 "/usr/lib64/ld-2.15.so"},
 		{(void*)0x35b1a21000, (void*)0x35b1a22000,
-		 PROT_READ | PROT_WRITE | MAP_PRIVATE, 0x20000, 0, 0,
+		 PROT_READ | PROT_WRITE | MAP_PRIVATE, 0, 0, 0,
 		 ""},
 	};
 
@@ -85,9 +85,25 @@ START_TEST (test_parse_fd)
 	}
 	
 	ret = child_mmap_get_fd(fd, &maps);
-
 	ck_assert_int_eq(ret, 0);
 
+	int i;
+	for(i=0; i<sizeof(checks)/sizeof(*checks);i++)
+	{
+		dprintf(2, "line %d\n", i);
+		child_mmap_t *check, *res;
+		check = &checks[i];
+		res = &maps.maps[i];
+		ck_assert_ptr_eq(check->start, res->start);
+		ck_assert_ptr_eq(check->stop, res->stop);
+		ck_assert_int_eq(check->perm, res->perm);
+		ck_assert_int_eq(check->offset, res->offset);
+		ck_assert_mem_eq(&check->device, &res->device, sizeof(check->device));
+		ck_assert_mem_eq(&check->inode, &res->inode, sizeof(check->inode));
+		ck_assert_str_eq(check->pathname, res->pathname);
+	}
+	ck_assert_int_eq(i, maps.size);
+
 	close(fd);
 }
 END_TEST