[dev] [ii] patch - some cleanup and a change or two

From: Evan Gates <evan.gates_AT_gmail.com>
Date: Wed, 12 Oct 2011 12:47:04 -0700

Hello,

Here's a patch with a bit of cleanup and a few changes. Feel free to pick and choose which parts of this patch you feel are a good idea (if any). I've annotated the changes below, and attached the patch.

_AT_@ -121,13 +121,10 @@
                 perror("ii: cannot allocate memory");
                 exit(EXIT_FAILURE);
         }
- if(!channels) channels = c;
- else {
- c->next = channels;
- channels = c;
- }
         c->fd = fd;
         c->name = strdup(name);
+ c->next = channels;
+ channels = c;

No need to have a special case for channels == NULL. If it is, whether or not we say c->next = channels, we end up with c->next == NULL because of the calloc() a few lines earlier.



_AT_@ -223,10 +220,12 @@
 }
 
 static void proc_channels_input(Channel *c, char *buf) {
- /* static char infile[256]; */
+ static char infile[256];
         char *p = NULL;
 
- if(buf[0] != '/' && buf[0] != 0) {
+ if(buf[0] == '\0')
+ return;
+ if(buf[0] != '/') {
                 proc_channels_privmsg(c->name, buf);
                 return;
         }

Do create infile[] so we can delete it later, this was a nice feature, an easy way to tell which channels we are in. As it is now you can write to a fifo for a channel we aren't in, I'd rather there be no fifo.

If the line we have is empty, don't do anything, get's rid of an annoying message in the server out file, and I think is more correct.



_AT_@ -235,17 +234,14 @@
                 case 'j':
                         p = strchr(&buf[3], ' ');
                         if(p) *p = 0;
+ add_channel(&buf[3]);
                         if((buf[3]=='#')||(buf[3]=='&')||(buf[3]=='+')||(buf[3]=='!')){
                                 if(p) snprintf(message, PIPE_BUF, "JOIN %s %s\r\n", &buf[3], p + 1); /* password protected channel */
                                 else snprintf(message, PIPE_BUF, "JOIN %s\r\n", &buf[3]);
- add_channel(&buf[3]);
                         }
- else {
- if(p){
- add_channel(&buf[3]);
- proc_channels_privmsg(&buf[3], p + 1);
- return;
- }
+ else if(p){
+ proc_channels_privmsg(&buf[3], p + 1);
+ return;
                         }
                         break;
                 case 't':

Move add_channel out of the if statements. Doing this makes us add the channel if it is a user and no message is specified. This way the user can "/j some_user" and the directory and in fifo will be created just as they would for a channel, instead of needing to do "/j some_user this is a message". Also change to else if, saves a few lines.


_AT_@ -275,10 +271,11 @@
                         else
                                 snprintf(message, PIPE_BUF,
                                                 "PART %s :ii - 500 SLOC are too much\r\n", c->name);
- write(irc, message, strlen(message));
+ if((c->name[0]=='#')||(c->name[0]=='&')||(c->name[0]=='+')||(c->name[0]=='!'))
+ write(irc, message, strlen(message));
                         close(c->fd);
- /*create_filepath(infile, sizeof(infile), c->name, "in");
- unlink(infile); */
+ create_filepath(infile, sizeof(infile), c->name, "in");
+ unlink(infile);
                         rm_channel(c);
                         return;
                         break;

Check that the channel we are leaving is indeed a channel, and not just a user. This avoids "no such channel" messages in the server out when we try to leave a user.

Do delete the in fifo as mentioned above.


_AT_@ -344,7 +341,7 @@
                 write(irc, message, strlen(message));
                 return;
         } else if(!argv[TOK_NICKSRV] || !argv[TOK_USER]) { /* server command */
- snprintf(message, PIPE_BUF, "%s%s", argv[TOK_ARG] ? argv[TOK_ARG] : "", argv[TOK_TEXT] ? argv[TOK_TEXT] : "");
+ snprintf(message, PIPE_BUF, "<%s> %s%s", host, argv[TOK_ARG] ? argv[TOK_ARG] : "", argv[TOK_TEXT] ? argv[TOK_TEXT] : "");
                 print_out(0, message);
                 return;
         } else if(!strncmp("ERROR", argv[TOK_CMD], 6))

This one is personal preference, add the host name for server messages so that they show up the same as messages in channels. I like this for consistency (a single script can be used to parse output from channels and the host), and so the server/host is immediately obvious without depending on checking the path.h


_AT_@ -358,6 +355,8 @@
                 argv[TOK_CHAN] = argv[TOK_TEXT];
                 snprintf(message, PIPE_BUF, "-!- %s(%s) has joined %s", argv[TOK_NICKSRV], argv[TOK_USER], argv[TOK_TEXT]);
         } else if(!strncmp("PART", argv[TOK_CMD], 5)) {
+ if(!strncmp(nick, argv[TOK_NICKSRV], sizeof(nick)))
+ return;
                 snprintf(message, PIPE_BUF, "-!- %s(%s) has left %s", argv[TOK_NICKSRV], argv[TOK_USER], argv[TOK_CHAN]);
         } else if(!strncmp("MODE", argv[TOK_CMD], 5))
                 snprintf(message, PIPE_BUF, "-!- %s changed mode/%s -> %s %s", argv[TOK_NICKSRV], argv[TOK_CMD + 1] ? argv[TOK_CMD + 1] : "" , argv[TOK_CMD + 2]? argv[TOK_CMD + 2] : "", argv[TOK_CMD + 3] ? argv[TOK_CMD + 3] : "");

If the PART message is us leaving a channel, don't print it. This allows us to actually leave the channel instead of receiving that message and creating the channel again.


_AT_@ -472,7 +471,6 @@
         }
         snprintf(nick, sizeof(nick), "%s", spw->pw_name);
         snprintf(prefix, sizeof(prefix),"%s/irc", spw->pw_dir);
- if (argc <= 1 || (argc == 2 && argv[1][0] == '-' && argv[1][1] == 'h')) usage();
 
         for(i = 1; (i + 1 < argc) && (argv[i][0] == '-'); i++) {
                 switch (argv[i][1]) {
_AT_@ -485,6 +483,7 @@
                         default: usage(); break;
                 }
         }
+ if(i != argc) usage();
         irc = tcpopen(port);
         if(!snprintf(path, sizeof(path), "%s/%s", prefix, host)) {
                 fprintf(stderr, "%s", "ii: path to irc directory too long\n");

Simpler check for correct arguments (default catches -h). Also running ii with no arguments (argc == 1) is ok as the irc dir, host, port, and nick, are all set with defaults.


If any of this seems wrong or seems to be a bad idea please let me know (as I'm using this code personally...)
-Evan

Received on Wed Oct 12 2011 - 21:47:04 CEST

This archive was generated by hypermail 2.3.0 : Wed Oct 12 2011 - 21:48:03 CEST