Ticket #53: 8bit-charset.diff

.bzrignore

@@ -12 +12 @@
 bitlbee.pc
 .gdb_history
 tests/check
+description-pak
+*.tgz
 *.gcda
 *.gcov
 *.gcno

Makefile

@@ -42 +42 @@
         $(MAKE) -C tests clean
 
 distclean: clean $(subdirs)
+        $(MAKE) -C tests distclean
         rm -f Makefile.settings config.h bitlbee.pc
         find . -name 'DEADJOE' -o -name '*.orig' -o -name '*.rej' -o -name '*~' -exec rm -f {} \;
-        $(MAKE) -C test distclean
 
 check: all
         $(MAKE) -C tests
@@ -110 +110 @@
 endif
 
 encode: crypting.c
-        $(CC) crypting.c protocols/md5.c $(CFLAGS) -o encode -DCRYPTING_MAIN $(CFLAGS) $(EFLAGS) $(LFLAGS)
+        $(CC) crypting.c lib/md5.c $(CFLAGS) -o encode -DCRYPTING_MAIN $(CFLAGS) $(EFLAGS) $(LFLAGS)
 
 decode: encode
         cp encode decode

account.c

@@ -49 +49 @@
         a->irc = irc;
         
         s = set_add( &a->set, "auto_connect", "true", set_eval_account, a );
-        s->flags |= ACC_SET_NOSAVE;
+        s->flags |= SET_NOSAVE;
         
         s = set_add( &a->set, "auto_reconnect", "true", set_eval_bool, a );
         
         s = set_add( &a->set, "password", NULL, set_eval_account, a );
-        s->flags |= ACC_SET_NOSAVE;
+        s->flags |= SET_NOSAVE;
         
         s = set_add( &a->set, "username", NULL, set_eval_account, a );
-        s->flags |= ACC_SET_NOSAVE | ACC_SET_OFFLINE_ONLY;
+        s->flags |= SET_NOSAVE | ACC_SET_OFFLINE_ONLY;
         set_setstr( &a->set, "username", user );
         
         a->nicks = g_hash_table_new_full( g_str_hash, g_str_equal, g_free, g_free );

account.h

@@ -52 +52 @@
 
 char *set_eval_account( set_t *set, char *value );
 
-#define ACC_SET_NOSAVE          1
 #define ACC_SET_OFFLINE_ONLY    2
 #define ACC_SET_ONLINE_ONLY     4
 

bitlbee.h

@@ -117 +117 @@
 
 extern char *CONF_FILE;
 
+#include "set.h"
 #include "irc.h"
 #include "storage.h"
-#include "set.h"
 #include "nogaim.h"
 #include "commands.h"
 #include "account.h"

configure

@@ -140 +140 @@
 fi
 
 if [ "$debug" = "1" ]; then
-        echo 'CFLAGS=-g' >> Makefile.settings
+        echo 'CFLAGS=-g -Wall -Werror' >> Makefile.settings
         echo 'DEBUG=1' >> Makefile.settings
         echo '#define DEBUG' >> config.h
 else

doc/user-guide/commands.xml

@@ -620 +620 @@
                 </description>
         </bitlbee-setting>
 
+        <bitlbee-setting name="8bit_charset_out" type="string" scope="account">
+                <default>iso8859-1</default>
+                <possible-values>you can get a list of all possible values by doing 'iconv -l' in a shell</possible-values>
+                <description>
+                        <para>
+                                If you want to behave like buggy ICQ client and send 8bit messages in some 8bit encoding and not in unicode, than you can modify this variable.
+                        </para>
+
+                        <para>
+                                BUT DON'T TOUCH THIS VARIABLE UNLESS YOU'RE SURE WHAT YOU'RE DOING. You've been wanred.
+                        </para>
+                </description>
+        </bitlbee-setting>
+
+        <bitlbee-setting name="8bit_charset_in" type="string" scope="account">
+                <default>iso8859-1</default>
+                <possible-values>you can get a list of all possible values by doing 'iconv -l' in a shell</possible-values>
+                <description>
+                        <para>
+                                Some ICQ clients send 8-bit messages in local 8-bit encoding (e.g. cp1251 for Russia) and not in unicode, but they mark the message as 8-bit, so we have to recode it properly.
+                        </para>
+
+                        <para>
+                                If you don't know what's the best value for this, at least iso8859-1 is the best choice for most Western countries. That's default one and that is usually used in ICQ protocol.
+                        </para>
+                </description>
+        </bitlbee-setting>
+
         <bitlbee-setting name="web_aware" type="string" scope="account">
                 <default>false</default>
 

log.c

@@ -26 +26 @@
 #define BITLBEE_CORE
 #include "bitlbee.h"
 #include <syslog.h>
+#include <assert.h>
 
 static log_t logoutput;
 
@@ -47 +48 @@
         return;
 }
 
-void log_link(int level, int output) {
-        /* I know it's ugly, but it works and I didn't feel like messing with pointer to function pointers */
+void log_link(loglvl_t level, logoutput_t output) {
+        log_somewhere_f log_func;
 
-        if(level == LOGLVL_INFO) {
-                if(output == LOGOUTPUT_NULL)
-                        logoutput.informational = &log_null;    
-                else if(output == LOGOUTPUT_IRC)
-                        logoutput.informational = &log_irc;     
-                else if(output == LOGOUTPUT_SYSLOG)
-                        logoutput.informational = &log_syslog;  
-                else if(output == LOGOUTPUT_CONSOLE) 
-                        logoutput.informational = &log_console; 
+        switch (output) {
+                case LOGOUTPUT_NULL:    log_func = log_null;    break;
+                case LOGOUTPUT_IRC:     log_func = log_irc;     break;
+                case LOGOUTPUT_SYSLOG:  log_func = log_syslog;  break;
+                case LOGOUTPUT_CONSOLE: log_func = log_console; break;
+                default: assert(0 == 1); return;
         }
-        else if(level == LOGLVL_WARNING) {
-                if(output == LOGOUTPUT_NULL)
-                        logoutput.warning = &log_null;
-                else if(output == LOGOUTPUT_IRC)
-                        logoutput.warning = &log_irc;
-                else if(output == LOGOUTPUT_SYSLOG)
-                        logoutput.warning = &log_syslog;
-                else if(output == LOGOUTPUT_CONSOLE)
-                        logoutput.warning = &log_console;
+
+        switch (level) {
+                case LOGLVL_INFO:    logoutput.informational = log_func; break;
+                case LOGLVL_WARNING: logoutput.warning       = log_func; break;
+                case LOGLVL_ERROR:   logoutput.error         = log_func; break;
+#ifdef DEBUG
+                case LOGLVL_DEBUG:   logoutput.debug         = log_func; break;
+#endif
+                default: assert(0 == 1); return;
         }
-        else if(level == LOGLVL_ERROR) {
-                if(output == LOGOUTPUT_NULL)
-                        logoutput.error = &log_null;
-                else if(output == LOGOUTPUT_IRC)
-                        logoutput.error = &log_irc;
-                else if(output == LOGOUTPUT_SYSLOG)
-                        logoutput.error = &log_syslog;
-                else if(output == LOGOUTPUT_CONSOLE)
-                        logoutput.error = &log_console;
-        }
-#ifdef DEBUG
-        else if(level == LOGLVL_DEBUG) {
-                if(output == LOGOUTPUT_NULL)
-                        logoutput.debug = &log_null;
-                else if(output == LOGOUTPUT_IRC)
-                        logoutput.debug = &log_irc;
-                else if(output == LOGOUTPUT_SYSLOG)
-                        logoutput.debug = &log_syslog;
-                else if(output == LOGOUTPUT_CONSOLE)
-                        logoutput.debug = &log_console;
-        }
-#endif
-        return; 
-
 }
 
-void log_message(int level, char *message, ... ) {
+void log_message(loglvl_t level, char *message, ... ) {
 
         va_list ap;
         char *msgstring;

log.h

@@ -42 +42 @@
         LOGOUTPUT_CONSOLE,
 } logoutput_t;
 
+typedef void (*log_somewhere_f)(int level, char *logmessage);
 typedef struct log_t {
-        void (*error)(int level, char *logmessage);
-        void (*warning)(int level, char *logmessage); 
-        void (*informational)(int level, char *logmessage);
+        log_somewhere_f error;
+        log_somewhere_f warning;
+        log_somewhere_f informational;
 #ifdef DEBUG
-        void (*debug)(int level, char *logmessage);
+        log_somewhere_f debug;
 #endif
 } log_t;
 
 void log_init(void);
-void log_link(int level, int output);
-void log_message(int level, char *message, ...) G_GNUC_PRINTF( 2, 3 );
+void log_link(loglvl_t level, logoutput_t output);
+void log_message(loglvl_t level, char *message, ...) G_GNUC_PRINTF( 2, 3 );
 void log_error(char *functionname);
 
 #endif

protocols/jabber/jabber.c

@@ -1506 +1506 @@
         s->flags |= ACC_SET_OFFLINE_ONLY;
         
         s = set_add( &acc->set, "server", NULL, set_eval_account, acc );
-        s->flags |= ACC_SET_NOSAVE | ACC_SET_OFFLINE_ONLY;
+        s->flags |= SET_NOSAVE | ACC_SET_OFFLINE_ONLY;
         
         s = set_add( &acc->set, "ssl", "false", set_eval_bool, acc );
         s->flags |= ACC_SET_OFFLINE_ONLY;

protocols/msn/msn.c

@@ -33 +33 @@
         set_t *s;
         
         s = set_add( &acc->set, "display_name", NULL, msn_set_display_name, acc );
-        s->flags |= ACC_SET_NOSAVE | ACC_SET_ONLINE_ONLY;
+        s->flags |= SET_NOSAVE | ACC_SET_ONLINE_ONLY;
 }
 
 static void msn_login( account_t *acc )

protocols/oscar/im.c

@@ -114 +114 @@
  *                        made up of UNICODE duples.  If you set
  *                        this, you'd better be damn sure you know
  *                        what you're doing.
- *   AIM_IMFLAGS_ISO_8859_1 -- The message contains the ASCII8 subset
- *                        known as ISO-8859-1.  
- *
+ *   AIM_IMFLAGS_LOCAL_8BIT -- The message contains some 8bit (it was
+ *                             _ISO_8859_1 before, but we have to support
+ *                             other 8bit encodings)
+ * 
  * Generally, you should use the lowest encoding possible to send
  * your message.  If you only use basic punctuation and the generic
  * Latin alphabet, use ASCII7 (no flags).  If you happen to use non-ASCII7
@@ -278 +279 @@
                 } else {
                         if (args->flags & AIM_IMFLAGS_UNICODE)
                                 aimbs_put16(&fr->data, 0x0002);
-                        else if (args->flags & AIM_IMFLAGS_ISO_8859_1)
+                        else if (args->flags & AIM_IMFLAGS_LOCAL_8BIT)
                                 aimbs_put16(&fr->data, 0x0003);
                         else
                                 aimbs_put16(&fr->data, 0x0000);
@@ -1213 +1214 @@
                         else if (args->charset == 0x0002)
                                 args->icbmflags |= AIM_IMFLAGS_UNICODE;
                         else if (args->charset == 0x0003)
-                                args->icbmflags |= AIM_IMFLAGS_ISO_8859_1;
+                                args->icbmflags |= AIM_IMFLAGS_LOCAL_8BIT;
                         else if (args->charset == 0xffff)
                                 ; /* no encoding (yeep!) */
 
@@ -1677 +1678 @@
 
         args.uin = aimbs_getle32(&meat);
         args.type = aimbs_getle16(&meat);
-        args.msg = (char *)aimbs_getraw(&meat, aimbs_getle16(&meat));
+        args.msglen = aimbs_getle16(&meat);
+        args.msg = (char *)aimbs_getraw(&meat, args.msglen);
 
         if ((userfunc = aim_callhandler(sess, rx->conn, snac->family, snac->subtype)))
                 ret = userfunc(sess, rx, channel, userinfo, &args);

protocols/oscar/im.h

@@ -19 +19 @@
 #define AIM_IMFLAGS_AWAY                0x0001 /* mark as an autoreply */
 #define AIM_IMFLAGS_ACK                 0x0002 /* request a receipt notice */
 #define AIM_IMFLAGS_UNICODE             0x0004
-#define AIM_IMFLAGS_ISO_8859_1          0x0008
+#define AIM_IMFLAGS_LOCAL_8BIT          0x0008
 #define AIM_IMFLAGS_BUDDYREQ            0x0010 /* buddy icon requested */
 #define AIM_IMFLAGS_HASICON             0x0020 /* already has icon */
 #define AIM_IMFLAGS_SUBENC_MACINTOSH    0x0040 /* damn that Steve Jobs! */
@@ -180 +180 @@
         guint32 uin; /* Of the sender of the ICBM */
         guint16 type;
         char *msg; /* Reason for auth request, deny, or accept */
+        size_t msglen;
 };
 
 int aim_send_rtfmsg(aim_session_t *sess, struct aim_sendrtfmsg_args *args);

protocols/oscar/oscar.c

@@ -360 +360 @@
         set_t *s;
         
         s = set_add( &acc->set, "server", NULL, set_eval_account, acc );
-        s->flags |= ACC_SET_NOSAVE | ACC_SET_OFFLINE_ONLY;
+        s->flags |= SET_NOSAVE | ACC_SET_OFFLINE_ONLY;
+
+        s = set_add( &acc->set, "8bit_charset_in", "ISO8859-1", set_eval_charset, acc );
+        s = set_add( &acc->set, "8bit_charset_out", "ISO8859-1", set_eval_charset, acc );
         
         if (isdigit(acc->user[0])) {
                 s = set_add( &acc->set, "web_aware", "false", set_eval_bool, acc );
@@ -1057 +1060 @@
         return 1;
 }
 
+static int is_8bit(char *buf, size_t buflen) {
+        int i;
+        for (i = 0; i < buflen; i++) {
+                if (buf[i] & 0x80)
+                        return 1;
+        }
+        return 0;
+}
+
+/** icbmflags:
+ * AIM_IMFLAGS_UNICODE      msg should be in UNICODEBIG
+ * AIM_IMFLAGS_LOCAL_8BIT   msg should be in "8bit_charset_in"
+ * other flags are ignored
+ */
+static void decode_incomingim(struct gaim_connection *gc, char *handle, char *msg, size_t msglen, guint32 icbmflags, char *out, size_t outlen)
+{
+        char msg_is_8bit = is_8bit(msg, msglen);
+
+        if ( msg_is_8bit && !(icbmflags & (AIM_IMFLAGS_UNICODE | AIM_IMFLAGS_LOCAL_8BIT)) )
+                if (set_getbool(&gc->irc->set, "debug"))
+                        serv_got_crap(gc, _("DEBUG: (%s): 8bit ascii message declared as 7bit one came..."), handle);
+        
+        if ( (icbmflags & AIM_IMFLAGS_UNICODE) || msg_is_8bit ) {
+                char *src;
+                
+                if (icbmflags & AIM_IMFLAGS_UNICODE)
+                        src = "UNICODEBIG"; // should we check for utf8 ?
+                else 
+                        src = set_getstr(&gc->acc->set, "8bit_charset_in");
+                
+                /* Try to use iconv first to convert the message to UTF8 - which is what BitlBee expects */
+                if (do_iconv(src, "UTF-8", msg, out, msglen, outlen) >= 0) {
+                        // Successfully converted!
+                } else if (icbmflags & AIM_IMFLAGS_UNICODE) {
+                        int i;
+                        
+                        for (i = 0, out[0] = '\0'; i < msglen; i += 2) {
+                                unsigned short uni;
+                                
+                                uni = ((msg[i] & 0xff) << 8) | (msg[i+1] & 0xff);
+        
+                                if ( uni < 128 ) { /* ASCII */
+                                        g_snprintf(out+strlen(out), outlen-strlen(out), "%c", uni);
+                                } else { /* something else, do UNICODE entity */
+                                        g_snprintf(out+strlen(out), outlen-strlen(out), "&#%04x;", uni);
+                                }
+                        }
+                } else {
+                        if (set_getbool(&gc->irc->set, "debug"))
+                                serv_got_crap(gc, _("DEBUG: (%s): Can't convert from %s to UTF-8, sending as-is"), handle, src);
+                        g_snprintf(out, outlen, "%s", msg);
+                }
+        } else {
+                g_snprintf(out, outlen, "%s", msg);
+        }
+}
+
 static int incomingim_chan1(aim_session_t *sess, aim_conn_t *conn, aim_userinfo_t *userinfo, struct aim_incomingim_ch1_args *args) {
         char *tmp = g_malloc(BUF_LONG + 1);
         struct gaim_connection *gc = sess->aux_data;
@@ -1064 +1124 @@
         
         if (args->icbmflags & AIM_IMFLAGS_AWAY)
                 flags |= IM_FLAG_AWAY;
-        
-        if ((args->icbmflags & AIM_IMFLAGS_UNICODE) || (args->icbmflags & AIM_IMFLAGS_ISO_8859_1)) {
-                char *src;
-                
-                if (args->icbmflags & AIM_IMFLAGS_UNICODE)
-                        src = "UNICODEBIG";
-                else
-                        src = "ISO8859-1";
-                
-                /* Try to use iconv first to convert the message to UTF8 - which is what BitlBee expects */
-                if (do_iconv(src, "UTF-8", args->msg, tmp, args->msglen, BUF_LONG) >= 0) {
-                        // Successfully converted!
-                } else if (args->icbmflags & AIM_IMFLAGS_UNICODE) {
-                        int i;
-                        
-                        for (i = 0, tmp[0] = '\0'; i < args->msglen; i += 2) {
-                                unsigned short uni;
-                                
-                                uni = ((args->msg[i] & 0xff) << 8) | (args->msg[i+1] & 0xff);
-        
-                                if ((uni < 128) || ((uni >= 160) && (uni <= 255))) { /* ISO 8859-1 */
-                                        g_snprintf(tmp+strlen(tmp), BUF_LONG-strlen(tmp), "%c", uni);
-                                } else { /* something else, do UNICODE entity */
-                                        g_snprintf(tmp+strlen(tmp), BUF_LONG-strlen(tmp), "&#%04x;", uni);
-                                }
-                        }
-                } else {
-                        g_snprintf(tmp, BUF_LONG, "%s", args->msg);
-                }
-        } else
-                g_snprintf(tmp, BUF_LONG, "%s", args->msg);
+
+        decode_incomingim(gc, userinfo->sn, args->msg, args->msglen, args->icbmflags, tmp, BUF_LONG);
         
         strip_linefeed(tmp);
         serv_got_im(gc, userinfo->sn, tmp, flags, time(NULL), -1);
@@ -1192 +1223 @@
                 case 0x0001: { /* An almost-normal instant message.  Mac ICQ sends this.  It's peculiar. */
                         char *uin, *message;
                         uin = g_strdup_printf("%u", args->uin);
-                        message = g_strdup(args->msg);
+                        message = g_malloc(BUF_LONG + 1);
+                        decode_incomingim(gc, uin, args->msg, args->msglen, 0, message, BUF_LONG);
                         strip_linefeed(message);
                         serv_got_im(gc, uin, message, 0, time(NULL), -1);
                         g_free(uin);
@@ -1787 +1819 @@
 
         switch (msg->type) {
                 case 0x0001: { /* Basic offline message */
-                        char sender[32];
-                        char *dialog_msg = g_strdup(msg->msg);
+                        char *uin = g_strdup_printf("%u", msg->sender);
+                        char *message = g_malloc(BUF_LONG + 1);
+                        int msglen = strlen(msg->msg);
                         time_t t = get_time(msg->year, msg->month, msg->day, msg->hour, msg->minute, 0);
-                        g_snprintf(sender, sizeof(sender), "%u", msg->sender);
-                        strip_linefeed(dialog_msg);
-                        serv_got_im(gc, sender, dialog_msg, 0, t, -1);
-                        g_free(dialog_msg);
+                        decode_incomingim(gc, uin, msg->msg, msglen, 0, message, BUF_LONG);
+                        strip_linefeed(message);
+                        serv_got_im(gc, uin, message, 0, t, -1);
+                        g_free(message);
+                        g_free(uin);
                 } break;
 
                 case 0x0004: { /* Someone sent you a URL */
@@ -1873 +1907 @@
                         s = g_malloc(BUF_LONG);
                         /* Try if we can put it in an ISO8859-1 string first.
                            If we can't, fall back to UTF16. */
-                        if ((ret = do_iconv("UTF-8", "ISO8859-1", message, s, len, BUF_LONG)) >= 0) {
-                                args.flags |= AIM_IMFLAGS_ISO_8859_1;
+                        if ((ret = do_iconv("UTF-8", set_getstr(&gc->acc->set, "8bit_charset_out"), message, s, len, BUF_LONG)) >= 0) {
+                                args.flags |= AIM_IMFLAGS_LOCAL_8BIT;
                                 len = ret;
                         } else if ((ret = do_iconv("UTF-8", "UNICODEBIG", message, s, len, BUF_LONG)) >= 0) {
                                 args.flags |= AIM_IMFLAGS_UNICODE;
@@ -2377 +2411 @@
         return ret;
 }
 
-static char *oscar_encoding_to_utf8(char *encoding, char *text, int textlen)
+static char *oscar_encoding_to_utf8(struct gaim_connection *gc, char *encoding, char *text, int textlen)
 {
         char *utf8 = g_new0(char, 8192);
 
         if ((encoding == NULL) || encoding[0] == '\0') {
-                /*              gaim_debug_info("oscar", "Empty encoding, assuming UTF-8\n");*/
+                if (set_getbool(&gc->irc->set, "debug"))
+                        serv_got_crap(gc, _("DEBUG: Empty encoding, assuming UTF-8"));
         } else if (!g_strcasecmp(encoding, "iso-8859-1")) {
                 do_iconv("iso-8859-1", "UTF-8", text, utf8, textlen, 8192);
         } else if (!g_strcasecmp(encoding, "ISO-8859-1-Windows-3.1-Latin-1")) {
@@ -2390 +2425 @@
         } else if (!g_strcasecmp(encoding, "unicode-2-0")) {
                 do_iconv("UCS-2BE", "UTF-8", text, utf8, textlen, 8192);
         } else if (g_strcasecmp(encoding, "us-ascii") && strcmp(encoding, "utf-8")) {
-                /*              gaim_debug_warning("oscar", "Unrecognized character encoding \"%s\", "
-                  "attempting to convert to UTF-8 anyway\n", encoding);*/
+                if (set_getbool(&gc->irc->set, "debug"))
+                        serv_got_crap(gc, _("DEBUG: Unrecognized character encoding \"%s\", attempting to convert to UTF-8 anyway"), encoding);
                 do_iconv(encoding, "UTF-8", text, utf8, textlen, 8192);
         }
 
@@ -2442 +2477 @@
                 }
                 
                 if(text) {
-                        utf8 = oscar_encoding_to_utf8(extracted_encoding, text, text_length);
+                        utf8 = oscar_encoding_to_utf8(gc, extracted_encoding, text, text_length);
                         serv_got_crap(gc, "%s\n%s", _("User Info"), utf8);
                 } else {
                         serv_got_crap(gc, _("No user info available."));
                 }
         } else if(infotype == AIM_GETINFO_AWAYMESSAGE && userinfo->flags & AIM_FLAG_AWAY) {
-                utf8 = oscar_encoding_to_utf8(extracted_encoding, text, text_length);
+                utf8 = oscar_encoding_to_utf8(gc, extracted_encoding, text, text_length);
                 serv_got_crap(gc, "%s\n%s", _("Away Message"), utf8);
         }
 
@@ -2541 +2576 @@
         /* Message contains high ASCII chars, time for some translation! */
         if (*s) {
                 s = g_malloc(BUF_LONG);
-                /* Try if we can put it in an ISO8859-1 string first.
+                /* Try if we can put it in an ISO8859-1 (or something that overrides it) string first.
                    If we can't, fall back to UTF16. */
-                if ((ret = do_iconv("UTF-8", "ISO8859-1", message, s, len, BUF_LONG)) >= 0) {
+                if ((ret = do_iconv("UTF-8", set_getstr(&gc->acc->set, "8bit_charset_out"), message, s, len, BUF_LONG)) >= 0) {
                         flags |= AIM_CHATFLAGS_ISO_8859_1;
                         len = ret;
                 } else if ((ret = do_iconv("UTF-8", "UNICODEBIG", message, s, len, BUF_LONG)) >= 0) {

root_commands.c

@@ -203 +203 @@
         }
 }
 
+
+static void setting_dump( irc_t *irc, char *key, char *value );
+
 static void cmd_account( irc_t *irc, char **cmd )
 {
         account_t *a;
@@ -407 +410 @@
                 if( set_name ) /* else 'forgotten' on purpose.. Must show new value after changing */
                 {
                         char *s = set_getstr( &a->set, set_name );
-                        if( s )
-                                irc_usermsg( irc, "%s = `%s'", set_name, s );
-                        else
-                                irc_usermsg( irc, "%s is empty", set_name );
+                        setting_dump(irc, set_name, s);
                 }
                 else
                 {
-                        set_t *s = a->set;
-                        while( s )
-                        {
-                                if( s->value || s->def )
-                                        irc_usermsg( irc, "%s = `%s'", s->key, s->value?s->value:s->def );
-                                else
-                                        irc_usermsg( irc, "%s is empty", s->key );
-                                s = s->next;
+                        set_t *set = a->set;
+                        for (set = a->set; set; set = set->next) {
+                                char *value = set->value ? set->value : set->def;
+                                setting_dump(irc, set->key, value);
                         }
                 }
                 
@@ -520 +516 @@
         }
 }
 
+// heler for cmd_rename
+static user_t* get_mutable_user( irc_t *irc, char *nick )
+{
+        user_t *u = NULL;
+        if( g_strcasecmp( nick, irc->nick ) == 0 )
+        {
+                irc_usermsg( irc, "Nick `%s' can't be changed", nick );
+        }
+        else if( !( u = user_find( irc, nick ) ) )
+        {
+                irc_usermsg( irc, "Nick `%s' does not exist", nick );
+        }
+        return u;
+}
+
 static void cmd_rename( irc_t *irc, char **cmd )
 {
         user_t *u;
         
-        if( g_strcasecmp( cmd[1], irc->nick ) == 0 )
-        {
-                irc_usermsg( irc, "Nick `%s' can't be changed", cmd[1] );
-        }
+        if( !( u = get_mutable_user(irc, cmd[1]) ))
+                ;
         else if( user_find( irc, cmd[2] ) && ( nick_cmp( cmd[1], cmd[2] ) != 0 ) )
         {
                 irc_usermsg( irc, "Nick `%s' already exists", cmd[2] );
@@ -535 +544 @@
         else if( !nick_ok( cmd[2] ) )
         {
                 irc_usermsg( irc, "Nick `%s' is invalid", cmd[2] );
-        }
-        else if( !( u = user_find( irc, cmd[1] ) ) )
-        {
-                irc_usermsg( irc, "Nick `%s' does not exist", cmd[1] );
         }
         else
         {
@@ -737 +742 @@
                 query_answer( irc, q, 0 );
 }
 
+// cmd_set and cmd_account(set) helper to remove code duplication
+static void setting_dump( irc_t *irc, char *key, char *value )
+{
+        if( value )
+                irc_usermsg( irc, "%s = `%s'", key, value );
+        else
+                irc_usermsg( irc, "%s is empty", key );
+}
+
 static void cmd_set( irc_t *irc, char **cmd )
 {
         if( cmd[1] && cmd[2] )
@@ -749 +763 @@
         if( cmd[1] ) /* else 'forgotten' on purpose.. Must show new value after changing */
         {
                 char *s = set_getstr( &irc->set, cmd[1] );
-                if( s )
-                        irc_usermsg( irc, "%s = `%s'", cmd[1], s );
-                else
-                        irc_usermsg( irc, "%s is empty", cmd[1] );
+                setting_dump(irc, cmd[1], s);
         }
         else
         {
                 set_t *s = irc->set;
                 while( s )
                 {
-                        if( s->value || s->def )
-                                irc_usermsg( irc, "%s = `%s'", s->key, s->value?s->value:s->def );
-                        else
-                                irc_usermsg( irc, "%s is empty", s->key );
+                        char *value = s->value ? s->value : s->def;
+                        setting_dump(irc, s->key, value);
                         s = s->next;
                 }
         }

set.h

@@ -23 +23 @@
   Suite 330, Boston, MA  02111-1307  USA
 */
 
+#ifndef _SET_H
+#define _SET_H
+
 /* This used to be specific to irc_t structures, but it's more generic now
    (so it can also be used for account_t structs). It's pretty simple, but
    so far pretty useful.
@@ -32 +35 @@
    from setting invalid values, you can write an evaluator function for
    every setting, which can check a new value and block it by returning
    NULL, or replace it by returning a new value. See struct set.eval. */
+
+struct set;
 
 typedef char *(*set_eval) ( struct set *set, char *value );
 
@@ -56 +61 @@
         set_eval eval;
         struct set *next;
 } set_t;
+
+#define SET_NOSAVE              1
 
 /* Should be pretty clear. */
 set_t *set_add( set_t **head, char *key, char *def, set_eval eval, void *data );
@@ -87 +94 @@
 char *set_eval_to_char( set_t *set, char *value );
 char *set_eval_ops( set_t *set, char *value );
 char *set_eval_charset( set_t *set, char *value );
+
+#endif

storage_xml.c

@@ -370 +370 @@
 
 static gboolean xml_save_nick( gpointer key, gpointer value, gpointer data );
 
+static int settings_save(int fd, int ident, set_t* set_start)
+{
+        set_t *set;
+        for( set = set_start; set; set = set->next )
+                if( set->value && !(set->flags & SET_NOSAVE) ) // I see no good reason to test for ``set->def``
+                        if( !xml_printf( fd, ident, "<setting name=\"%s\">%s</setting>\n", set->key, set->value ) )
+                                return -1;
+        return 0;
+}
+
 static storage_status_t xml_save( irc_t *irc, int overwrite )
 {
         char path[512], *path2, *pass_buf = NULL;
-        set_t *set;
         account_t *acc;
         int fd;
         md5_byte_t pass_md5[21];
@@ -416 +425 @@
         
         g_free( pass_buf );
         
-        for( set = irc->set; set; set = set->next )
-                if( set->value && set->def )
-                        if( !xml_printf( fd, 1, "<setting name=\"%s\">%s</setting>\n", set->key, set->value ) )
-                                goto write_error;
+        if ( settings_save(fd, 1, irc->set) < 0)
+                goto write_error;
         
         for( acc = irc->accounts; acc; acc = acc->next )
         {
@@ -442 +449 @@
                 if( !xml_printf( fd, 0, ">\n" ) )
                         goto write_error;
                 
-                for( set = acc->set; set; set = set->next )
-                        if( set->value && set->def && !( set->flags & ACC_SET_NOSAVE ) )
-                                if( !xml_printf( fd, 2, "<setting name=\"%s\">%s</setting>\n", set->key, set->value ) )
-                                        goto write_error;
+                if ( settings_save(fd, 2, acc->set) < 0)
+                        goto write_error;
                 
                 /* This probably looks pretty strange. g_hash_table_foreach
                    is quite a PITA already (but it can't get much better in

tests/Makefile

@@ -1 @@
-include ../Makefile.settings
+-include ../Makefile.settings
 
 LFLAGS +=-lcheck
 
@@ -7 +7 @@
 
 clean:
         rm -f check *.o
+
+distclean: clean
 
 main_objs = account.o bitlbee.o conf.o crypting.o help.o ipc.o irc.o irc_commands.o log.o nick.o query.o root_commands.o set.o storage.o storage_xml.o storage_text.o user.o