From 8a446f43ff184fad6f7aeca05fca51bb3aac2d91 Mon Sep 17 00:00:00 2001
From: Jan Kanis <jan.code@jankanis.nl>
Date: Sat, 22 Dec 2012 18:38:28 +0100
Subject: [PATCH] include fixes and suggestions from code review

---
 builtin.cpp        | 10 +++++-----
 event.cpp          | 24 ++++++++++++++++--------
 event.h            |  8 +-------
 parser.cpp         |  4 ++--
 parser.h           |  4 ++--
 tests/test9.err    |  1 +
 tests/test9.in     |  3 +++
 tests/test9.status |  2 +-
 8 files changed, 31 insertions(+), 25 deletions(-)

diff --git a/builtin.cpp b/builtin.cpp
index 30421de3d..7df552e26 100644
--- a/builtin.cpp
+++ b/builtin.cpp
@@ -1010,12 +1010,12 @@ static int builtin_emit(parser_t &parser, wchar_t **argv)
 
     }
 
-    wcstring_list_t args;
-    wchar_t *eventname = argv[woptind];
-    for (woptind++; woptind < argc; woptind++)
-    {
-        args.push_back(argv[woptind]);
+    if(!argv[woptind]) {
+      append_format(stderr_buffer, L"%ls: expected event name\n", argv[0]);
+      return STATUS_BUILTIN_ERROR;
     }
+    wchar_t *eventname = argv[woptind];
+    wcstring_list_t args(argv + woptind + 1, argv + argc);
     event_fire_generic(eventname, &args);
 
     return STATUS_BUILTIN_OK;
diff --git a/event.cpp b/event.cpp
index 196296a0d..496eb337e 100644
--- a/event.cpp
+++ b/event.cpp
@@ -221,8 +221,11 @@ static void show_all_handlers(void)
 }
 #endif
 
-  
-wcstring event_type_str(const event_t &event) {
+/*
+    Give a more condensed description of \c event compared to \c event_get_desc. 
+    It includes what function will fire if the \c event is an event handler. 
+ */
+static wcstring event_desc_compact(const event_t &event) {
 	wcstring res;
 	wchar_t const *temp;
 	int sig; 
@@ -288,8 +291,10 @@ void event_add_handler(const event_t &event)
 {
     event_t *e;
 
-    if(debug_level >= 3)
-        debug(3, "register: %ls\n", event_type_str(event).c_str());
+    if(debug_level >= 3) {
+        wcstring desc = event_desc_compact(event);
+        debug(3, "register: %ls\n", desc.c_str());
+    }
 
     e = new event_t(event);
 
@@ -304,14 +309,16 @@ void event_add_handler(const event_t &event)
     signal_unblock();
 }
 
-void event_remove(event_t &criterion)
+void event_remove(const event_t &criterion)
 {
 
     size_t i;
     event_list_t new_list;
 
-		if(debug_level >= 3)
-				debug(3, "unregister: %ls\n", event_type_str(criterion).c_str());
+    if(debug_level >= 3) {
+        wcstring desc = event_desc_compact(criterion);
+        debug(3, "unregister: %ls\n", desc.c_str());
+    }
 
     /*
       Because of concurrency issues (env_remove could remove an event
@@ -504,7 +511,7 @@ static void event_fire_internal(const event_t &event)
         prev_status = proc_get_last_status();
         parser_t &parser = parser_t::principal_parser();
 
-        block_t *block = new event_block_t((const event_t*)&event);
+        block_t *block = new event_block_t(event);
         parser.push_block(block);
         parser.eval(buffer, io_chain_t(), TOP);
         parser.pop_block();
@@ -570,6 +577,7 @@ static void event_fire_delayed()
           Set up
         */
         event_t e = event_t::signal_event(0);
+        e.arguments.resize(1);
         lst = &sig_list[1-active_list];
 
         if (lst->overflow)
diff --git a/event.h b/event.h
index 1c87488e3..cd3bb0e31 100644
--- a/event.h
+++ b/event.h
@@ -109,7 +109,7 @@ void event_add_handler(const event_t &event);
 
    May not be called by a signal handler, since it may free allocated memory.
 */
-void event_remove(event_t &event);
+void event_remove(const event_t &event);
 
 /**
    Return all events which match the specified event class
@@ -171,12 +171,6 @@ void event_free(event_t *e);
 */
 wcstring event_get_desc(const event_t &e);
 
-/**
-   Returns a string describing the event. This version is more concise and
-   more detailed than event_get_desc.
-*/
-wcstring event_type_str(const event_t &e);
-
 /**
    Fire a generic event with the specified name
 */
diff --git a/parser.cpp b/parser.cpp
index 1fe7bd83e..a8da7ba08 100644
--- a/parser.cpp
+++ b/parser.cpp
@@ -893,7 +893,7 @@ void parser_t::stack_trace(block_t *b, wcstring &buff)
           This is an event handler
         */
         const event_block_t *eb = static_cast<const event_block_t *>(b);
-        wcstring description = event_get_desc(*eb->event);
+        wcstring description = event_get_desc(eb->event);
         append_format(buff, _(L"in event handler: %ls\n"), description.c_str());
         buff.append(L"\n");
 
@@ -3798,7 +3798,7 @@ if_block_t::if_block_t() :
 {
 }
 
-event_block_t::event_block_t(const event_t *evt) :
+event_block_t::event_block_t(const event_t &evt) :
     block_t(EVENT),
     event(evt)
 {
diff --git a/parser.h b/parser.h
index 751182c35..b956be4d5 100644
--- a/parser.h
+++ b/parser.h
@@ -158,8 +158,8 @@ struct if_block_t : public block_t
 
 struct event_block_t : public block_t
 {
-    const event_t * const event;
-    event_block_t(const event_t *evt);
+    event_t const &event;
+    event_block_t(const event_t &evt);
 };
 
 struct function_block_t : public block_t
diff --git a/tests/test9.err b/tests/test9.err
index e69de29bb..f439e8dba 100644
--- a/tests/test9.err
+++ b/tests/test9.err
@@ -0,0 +1 @@
+emit: expected event name
diff --git a/tests/test9.in b/tests/test9.in
index 688dfb3f2..b88427278 100644
--- a/tests/test9.in
+++ b/tests/test9.in
@@ -23,3 +23,6 @@ function test3 --on-event test3
 end
 
 emit test3 foo bar
+
+# test empty argument
+emit
\ No newline at end of file
diff --git a/tests/test9.status b/tests/test9.status
index 573541ac9..d00491fd7 100644
--- a/tests/test9.status
+++ b/tests/test9.status
@@ -1 +1 @@
-0
+1