broken plugin code

So, I've been using 5.3.x for a while, and am trying out the new 5.6.6.
I'm rather dumbfounded to find the new, more elaborate, plugin support.
I fail to see what it buys you that you can't get simply by wrapping
your plugin in a shell script and using $1 and $2 to good effect:

E.g., instead of plugin "/usr/local/ssl/bin/openssl s_client -quiet -connect 
%h:%p"

just use plugin /usr/local/bin/ssl_connect:

$ cat /usr/local/bin/ssl_connect 
#!/bin/sh
/usr/local/ssl/bin/openssl s_client -quiet -connect $1:$2

which is what I've been doing up till now.

Furthermore, there's actually a pretty bad bug in the plugin code
which causes it to SIGSEGV on my machine.  I'm surprised that it works
for anyone.  I suggest killing the new plugin stuff entirely, as it
seems to violate the "No adding code to do things that can just as
easily be done with wrappers" guideline.  Below is a patch to do that.


Todd

p.s.  I did track down the problem, so I'm sure it's localized to this
function and this doesn't just hide it.  If you're inclined to leave
this stuff in, I've included a note at the very bottom of this mail
explaining what's wrong.


--- fetchmail-5.6.6/socket.c    Sat Feb 10 16:14:58 2001
+++ fetchmail-5.6.6-tas/socket.c        Sat Feb 17 17:37:11 2001
@@ -71,75 +71,6 @@
 #endif /* NET_SECURITY */
 
 #ifdef HAVE_SOCKETPAIR
-char *const *parse_plugin(const char *plugin, const char *host, const char 
*service)
-{      const char **argvec;
-       const char *c, *p;
-       char *cp, *plugin_copy;
-       unsigned int plugin_copy_len;
-       unsigned int plugin_offset = 0, plugin_copy_offset = 0;
-       unsigned int i, s = 2 * sizeof(char*), host_count = 0, service_count = 
0;
-       unsigned int plugin_len = strlen(plugin);
-       unsigned int host_len = strlen(host);
-       unsigned int service_len = strlen(service);
-
-       for (c = p = plugin; *c; c++)
-       {       if (isspace(*c) && !isspace(*p))
-                       s += sizeof(char*);
-               if (*p == '%' && *c == 'h')
-                       host_count++;
-               if (*p == '%' && *c == 'p')
-                       service_count++;
-               p = c;
-       }
-
-       plugin_copy_len = plugin_len + host_len * host_count + service_len * 
service_count;
-       plugin_copy = malloc(plugin_copy_len + 1);
-       if (!plugin_copy)
-       {
-               report(stderr, _("fetchmail: malloc failed\n"));
-               return NULL;
-       }
-
-       while (plugin_copy_offset < plugin_copy_len)
-       {       if ((plugin[plugin_offset] == '%') && (plugin[plugin_offset + 
1] == 'h'))
-               {       strcat(plugin_copy + plugin_copy_offset, host);
-                       plugin_offset += 2;
-                       plugin_copy_offset += host_len;
-               }
-               else if ((plugin[plugin_offset] == '%') && 
(plugin[plugin_offset + 1] == 'p'))
-               {       strcat(plugin_copy + plugin_copy_offset, service);
-                       plugin_offset += 2;
-                       plugin_copy_offset += service_len;
-               }
-               else
-               {       plugin_copy[plugin_copy_offset] = plugin[plugin_offset];
-                       plugin_offset++;
-                       plugin_copy_offset++;
-               }
-       }
-       plugin_copy[plugin_copy_len] = 0;
-
-       argvec = malloc(s);
-       if (!argvec)
-       {
-               report(stderr, _("fetchmail: malloc failed\n"));
-               return NULL;
-       }
-       memset(argvec, 0, s);
-       for (c = p = plugin_copy, i = 0; *c; c++)
-       {       if ((!isspace(*c)) && (c == p ? 1 : isspace(*p))) {
-                       argvec[i] = c;
-                       i++;
-               }
-               p = c;
-       }
-       for (cp = plugin_copy; *cp; cp++)
-       {       if (isspace(*cp))
-                       *cp = 0;
-       }
-       return (char *const*)argvec;
-}
-
 static int handle_plugin(const char *host,
                         const char *service, const char *plugin)
 /* get a socket mediated through a given external command */
@@ -175,9 +106,8 @@
                (void) close(fds[0]);
                if (outlevel >= O_VERBOSE)
                    report(stderr, _("running %s (host %s service %s)\n"), 
plugin, host, service);
-               argvec = parse_plugin(plugin,host,service);
-               execvp(*argvec, argvec);
-               report(stderr, _("execvp(%s) failed\n"), *argvec);
+               execlp(plugin,plugin,host,service,0);
+               report(stderr, _("execl(%s) failed\n"), plugin);
                exit(0);
                break;
        default:        /* parent */





























































































The strcat (plugin_copy + plugin_copy_offset... stuff should really be
    strcpy .....  See?

Now, are you sure you really want to maintain this boilerplate?  :)