Hi!

The attached patch removes the give_feedback function by a new a litte more
flexible echo_message function. The give_feedback function was nearly unused
in the code. But I often found code like

Arg a;
a.i = Info;
a.s = g_strdup_printf("Message %s", message);
echo(&a);
g_free(a.s);

so I removed this code by a call to the new function too. This makes the code
a little smaller and also the compiled binary (on my machine by 3% or 4066
bytes). I'm not sure if I forgot some calls to free, but the check with
valgrind caused nearly the same output like the vimprobable version from
current master, so I'm confident that the patch works.

Comments are welcome!

Daniel
From 5ae03327f76887a8e51514dac7d7e69aedfaddd8 Mon Sep 17 00:00:00 2001
From: Daniel Carl <[email protected]>
Date: Fri, 7 Sep 2012 00:14:04 +0200
Subject: [PATCH] Added function to echo messages of different types.

The new function echo_message() makes the code easier to read. We don't need
to write multiple times code like:

Arg a;
a.i = Info;
a.s = g_strdup_printf("Message %s", message);
echo(&a);
g_free(a.s);

That makes the code complex.
By the way is the generated binary 4064 bytes smaller (~3%) with this patch.
---
 main.c        |  123 ++++++++++++++++-----------------------------------------
 utilities.c   |   28 ++++++-------
 utilities.h   |    2 +-
 vimprobable.h |    8 +++-
 4 files changed, 53 insertions(+), 108 deletions(-)

diff --git a/main.c b/main.c
index b3407ba..17d38e9 100644
--- a/main.c
+++ b/main.c
@@ -316,13 +316,10 @@ webview_download_cb(WebKitWebView *webview, WebKitDownload *download, gpointer u
     webkit_download_set_destination_uri(download, uri);
     g_free(uri);
     size = (uint32_t)webkit_download_get_total_size(download);
-    a.i = Info;
     if (size > 0)
-        a.s = g_strdup_printf("Download %s started (expected size: %u bytes)...", filename, size);
+        echo_message(Info, "Download %s started (expected size: %u bytes)...", filename, size);
     else
-        a.s = g_strdup_printf("Download %s started (unknown size)...", filename);
-    echo(&a);
-    g_free(a.s);
+        echo_message(Info, "Download %s started (unknown size)...", filename);
     activeDownloads = g_list_prepend(activeDownloads, download);
     g_signal_connect(download, "notify::progress", G_CALLBACK(download_progress), NULL);
     g_signal_connect(download, "notify::status", G_CALLBACK(download_progress), NULL);
@@ -345,15 +342,10 @@ download_progress(WebKitDownload *d, GParamSpec *pspec) {
 
     if (status != WEBKIT_DOWNLOAD_STATUS_STARTED && status != WEBKIT_DOWNLOAD_STATUS_CREATED) {
         if (status != WEBKIT_DOWNLOAD_STATUS_FINISHED) {
-            a.i = Error;
-            a.s = g_strdup_printf("Error while downloading %s", webkit_download_get_suggested_filename(d));
-            echo(&a);
+            echo_message(Error, "Error while downloading %s", webkit_download_get_suggested_filename(d));
         } else {
-            a.i = Info;
-            a.s = g_strdup_printf("Download %s finished", webkit_download_get_suggested_filename(d));
-            echo(&a);
+            echo_message(Info, "Download %s finished", webkit_download_get_suggested_filename(d));
         }
-        g_free(a.s);
         activeDownloads = g_list_remove(activeDownloads, d);
     }
     update_state();
@@ -405,9 +397,7 @@ webview_keypress_cb(WebKitWebView *webview, GdkEventKey *event) {
     case ModeNormal:
         if ((CLEAN(event->state) & ~irrelevant) == 0) {
             if (IS_ESCAPE(event)) {
-                a.i = Info;
-                a.s = g_strdup("");
-                echo(&a);
+                echo_message(Info, "");
                 g_free(a.s);
             } else if (current_modkey == 0 && ((event->keyval >= GDK_1 && event->keyval <= GDK_9)
                     || (event->keyval == GDK_0 && count))) {
@@ -435,13 +425,13 @@ webview_keypress_cb(WebKitWebView *webview, GdkEventKey *event) {
         }
     case ModePassThrough:
         if (IS_ESCAPE(event)) {
-            echo(&a);
+            echo_message(Info, "");
             set(&a);
             return TRUE;
         }
         break;
     case ModeSendKey:
-        echo(&a);
+        echo_message(Info, "");
         set(&a);
         break;
     }
@@ -1284,10 +1274,8 @@ yank(const Arg *arg) {
         if (!content && arg->i & ClipboardGTK)
             content = gtk_clipboard_wait_for_text(clipboards[1]);
         if (content) {
-            feedback = g_strconcat("Yanked ", content, NULL);
+            echo_message(Info, "Yanked %s", content);
             g_free((gpointer *)content);
-            give_feedback(feedback);
-            g_free((gpointer *)feedback);
         }
     } else {
         if (arg->i & SourceURL) {
@@ -1297,8 +1285,8 @@ yank(const Arg *arg) {
         }
         if (!url)
             return TRUE;
-        feedback = g_strconcat("Yanked ", url, NULL);
-        give_feedback(feedback);
+
+        echo_message(Info, "Yanked %s", url);
         if (arg->i & ClipboardPrimary)
             gtk_clipboard_set_text(clipboards[0], url, -1);
         if (arg->i & ClipboardGTK)
@@ -1400,12 +1388,9 @@ search(const Arg *arg) {
             if (arg->i & Wrapping) {
                 success = webkit_web_view_search_text(webview, search_handle, arg->i & CaseSensitive, direction, TRUE);
                 if (success) {
-                    a.i = Warning;
-                    a.s = g_strdup_printf("search hit %s, continuing at %s",
+                    echo_message(Warning, "search hit %s, continuing at %s",
                             direction ? "BOTTOM" : "TOP",
                             direction ? "TOP" : "BOTTOM");
-                    echo(&a);
-                    g_free(a.s);
                 } else
                     break;
             } else
@@ -1413,18 +1398,13 @@ search(const Arg *arg) {
         }
     } while(--count);
     if (!success) {
-        a.i = Error;
-        a.s = g_strdup_printf("Pattern not found: %s", search_handle);
-        echo(&a);
-        g_free(a.s);
+        echo_message(Error, "Pattern not found: %s", search_handle);
     }
     return TRUE;
 }
 
 gboolean
 set(const Arg *arg) {
-    Arg a = { .i = Info | NoAutoHide };
-
     switch (arg->i) {
     case ModeNormal:
         if (search_handle) {
@@ -1435,19 +1415,13 @@ set(const Arg *arg) {
         gtk_widget_grab_focus(GTK_WIDGET(webview));
         break;
     case ModePassThrough:
-        a.s = g_strdup("-- PASS THROUGH --");
-        echo(&a);
-        g_free(a.s);
+        echo_message(Info | NoAutoHide, "-- PASS THROUGH --");
         break;
     case ModeSendKey:
-        a.s = g_strdup("-- PASS TROUGH (next) --");
-        echo(&a);
-        g_free(a.s);
+        echo_message(Info | NoAutoHide, "-- PASS TROUGH (next) --");
         break;
     case ModeInsert: /* should not be called manually but automatically */
-        a.s = g_strdup("-- INSERT --");
-        echo(&a);
-        g_free(a.s);
+        echo_message(Info | NoAutoHide, "-- INSERT --");
         break;
     default:
         return TRUE;
@@ -1506,15 +1480,12 @@ quickmark(const Arg *a) {
         }
         char *ptr = strrchr(buf, '\n');
         *ptr = '\0';
-        Arg x = { .s = buf };
-        if (strlen(buf)) 
+        if (strlen(buf)) {
+            Arg x = { .s = buf };
             return open_arg(&x);
-        else {       
-            x.i = Error;
-            x.s = g_strdup_printf("Quickmark %d not defined", b);
-            echo(&x);
-            g_free(x.s);
-            return false; 
+        } else {
+            echo_message(Error, "Quickmark %d not defined", b);
+            return false;
         }
     } else { return false; }
 }
@@ -1539,10 +1510,7 @@ script(const Arg *arg) {
     }
     g_free(message);
     if (arg->i != Silent && value) {
-        a.i = arg->i;
-        a.s = g_strdup(value);
-        echo(&a);
-        g_free(a.s);
+        echo_message(arg->i, value);
     }
     /* switch mode according to scripts return value */
     if (value) {
@@ -1631,13 +1599,9 @@ fake_key_event(const Arg *a) {
     if(!embed) {
         return FALSE;
     }
-    Arg err;
-    err.i = Error;
     Display *xdpy;
     if ( (xdpy = XOpenDisplay(NULL)) == NULL ) {
-        err.s = g_strdup("Couldn't find the XDisplay.");
-        echo(&err);
-        g_free(err.s);
+        echo_message(Error, "Couldn't find the XDisplay.");
         return FALSE;
     }
        
@@ -1651,32 +1615,24 @@ fake_key_event(const Arg *a) {
     xk.state =  a->i;
 
     if( ! a->s ) {
-        err.s = g_strdup("Zero pointer as argument! Check your config.h");
-        echo(&err);
-        g_free(err.s);
+        echo_message(Error, "Zero pointer as argument! Check your config.h");
         return FALSE;
     }
 
     KeySym keysym;
     if( (keysym = XStringToKeysym(a->s)) == NoSymbol ) {
-        err.s = g_strdup_printf("Couldn't translate %s to keysym", a->s );
-        echo(&err);
-        g_free(err.s);
+        echo_message(Error, "Couldn't translate %s to keysym", a->s );
         return FALSE;
     }
     
     if( (xk.keycode = XKeysymToKeycode(xdpy, keysym)) == NoSymbol ) {
-        err.s = g_strdup("Couldn't translate keysym to keycode");
-        echo(&err);
-        g_free(err.s);
+        echo_message(Error, "Couldn't translate keysym to keycode");
         return FALSE;
     }
    
     xk.type = KeyPress;
     if( !XSendEvent(xdpy, embed, True, KeyPressMask, (XEvent *)&xk) ) {
-        err.s = g_strdup("XSendEvent failed");
-        echo(&err);
-        g_free(err.s);
+        echo_message(Error, "XSendEvent failed");
         return FALSE;
     }
     XFlush(xdpy);
@@ -1729,7 +1685,7 @@ bookmark(const Arg *arg) {
         }
         fprintf(f, "%s", "\n");
         fclose(f);
-        give_feedback( "Bookmark saved" );
+        echo_message(Info, "Bookmark saved");
         return TRUE;
     } else {
     	set_error("Bookmarks file not found.");
@@ -1935,7 +1891,7 @@ process_set_line(char *line) {
                     browsersettings[i].var[MAX_SETTING_SIZE - 1] = '\0';
                     /* in case this string is also used for a webkit setting, make sure it's consistent */
                     my_pair.value[MAX_SETTING_SIZE - 1] = '\0';
-                    give_feedback("String too long; automatically truncated!");
+                    echo_message(Info, "String too long; automatically truncated!");
                 }
             }
             if (strlen(browsersettings[i].webkit) > 0) {
@@ -2027,21 +1983,15 @@ process_line(char *line) {
     g_free(command_hist);
 
     if (!found) {
-        a.i = Error;
-        a.s = g_strdup_printf("Not a browser command: %s", c);
-        echo(&a);
-        g_free(a.s);
+        echo_message(Error, "Not a browser command: %s", c);
     } else if (!success) {
-        a.i = Error;
         if (error_msg != NULL) {
-            a.s = g_strdup_printf("%s", error_msg);
+            echo_message(Error, error_msg);
             g_free(error_msg);
             error_msg = NULL;
         } else {
-            a.s = g_strdup_printf("Unknown error. Please file a bug report!");
+            echo_message(Error, "Unknown error. Please file a bug report!");
         }
-        echo(&a);
-        g_free(a.s);
     }
     return success;
 }
@@ -2649,21 +2599,14 @@ main(int argc, char *argv[]) {
      * command line.
      */
     if (!(access(configfile, F_OK) == 0) && cfile) {
-	    char *feedback_str;
-
-	    feedback_str = g_strdup_printf("Config file '%s' doesn't exist", cfile);
-	    give_feedback(feedback_str);
-        g_free(feedback_str);
+        echo_message(Info, "Config file '%s' doesn't exist", cfile);
     } else if ((access(configfile, F_OK) == 0))
 	    configfile_exists = true;
 
     /* read config file */
     /* But only report errors if we failed, and the file existed. */
     if ((SUCCESS != read_rcfile(configfile)) && configfile_exists) {
-        a.i = Error;
-        a.s = g_strdup_printf("Error in config file '%s'", configfile);
-        echo(&a);
-        g_free(a.s);
+        echo_message(Error, "Error in config file '%s'", configfile);
         g_free(configfile);
     }
 
diff --git a/utilities.c b/utilities.c
index 6ee63d1..139f17c 100644
--- a/utilities.c
+++ b/utilities.c
@@ -49,15 +49,10 @@ process_save_qmark(const char *bm, WebKitWebView *webview)
     char qmarks[10][101];
     char buf[100];
     int  i, mark, l=0;
-    Arg a;
     mark = -1;
     mark = atoi(bm);
-    if ( mark < 1 || mark > 9 ) 
-    {
-	    a.i = Error;
-	    a.s = g_strdup_printf("Invalid quickmark, only 1-9");
-	    echo(&a);
-	    g_free(a.s);
+    if ( mark < 1 || mark > 9 ) {
+	    echo_message(Error, "Invalid quickmark, only 1-9");
 	    return TRUE;
     }	    
     if ( uri == NULL ) return FALSE;
@@ -92,10 +87,7 @@ process_save_qmark(const char *bm, WebKitWebView *webview)
     for( i=0; i < 10; ++i ) 
         fprintf(fp, "%s\n", qmarks[i]);
     fclose(fp);
-    a.i = Error;
-    a.s = g_strdup_printf("Saved as quickmark %d: %s", mark, uri);
-    echo(&a);
-    g_free(a.s);
+    echo_message(Error, "Saved as quickmark %d: %s", mark, uri);
 
     return TRUE;
 }
@@ -490,14 +482,18 @@ set_error(const char *error) {
     }
 }
 
-void 
-give_feedback(const char *feedback) 
-{ 
-    Arg a = { .i = Info };
+void
+echo_message(const MessageType type, const char *format, ...)
+{
+    Arg a;
+    va_list ap;
 
-    a.s = g_strdup_printf("%s", feedback);
+    va_start(ap, format);
+    a.i = type;
+    a.s = g_strdup_vprintf(format, ap);
     echo(&a);
     g_free(a.s);
+    va_end(ap);
 }
 
 Listelement *
diff --git a/utilities.h b/utilities.h
index f9ac1ba..b904e03 100644
--- a/utilities.h
+++ b/utilities.h
@@ -25,7 +25,7 @@ gboolean changemapping(Key *search_key, int maprecord, char *cmd);
 gboolean process_line_arg(const Arg *arg);
 gboolean build_taglist(const Arg *arg, FILE *f);
 void set_error(const char *error);
-void give_feedback(const char *feedback);
+void echo_message(const MessageType type, const char *format, ...);
 Listelement * complete_list(const char *searchfor, const int mode, Listelement *elementlist);
 Listelement * add_list(const char *element, Listelement *elementlist);
 int count_list(Listelement *elementlist);
diff --git a/vimprobable.h b/vimprobable.h
index 5a6c2df..b313508 100644
--- a/vimprobable.h
+++ b/vimprobable.h
@@ -70,7 +70,13 @@ enum { ZoomText, ZoomFullContent = (1 << 2) };
     relevant for script:
     1 << 3:                         1 = Silent (no echo)
 */
-enum { Info, Warning, Error, NoAutoHide = 1 << 2, Silent = 1 << 3 };
+typedef enum {
+    Info,
+    Warning,
+    Error,
+    NoAutoHide = 1 << 2,
+    Silent = 1 << 3
+} MessageType;
 enum { NthSubdir, Rootdir };
 enum { InsertCurrentURL = 1 };
 enum { Increment, Decrement };
-- 
1.7.9.5

Attachment: signature.asc
Description: Digital signature

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
Vimprobable-users mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/vimprobable-users

Reply via email to