Re: Compilation problems and bugs in wackamole 2.1.1

Richard Lloyd wrote:

>I just downloaded wackamole 2.1.1 (I've already installed spread 3.17.2
>of course and got that working) and went about compiling it for Fedora Core 2.
>  
>
Everyone on Linux would have the same problem.  As far as I know, 
wackamole on Linux isn't wildly popular.  FreeBSD is much more common -- 
which, I suppose, is why it took until now for someone to notice the 
glaring errors (excepting the getopt in wackatrl which effected everyone).

>Here's the problems I found:
>
>* ife-sockpacket.c has a reference to ping_pkt (lines 87 and 88), but
>  this isn't defined anywhere. My guess is that whoever added that code
>  has a distro whose system includes define ping_pkt, but that is not
>  the case with FC2. My solution was to simply put in a local ping_pkt
>  variable (maybe it should be called something else in case that clashes
>  with the original author's distro?) with a reasonable char array size
>  (hey, I wasn't going to work it out exactly, see the actual compose_ping()
>  function to see why :-) ):
>
>    if (icmp) {
>      unsigned char ping_pkt[255];
>      compose_ping(ping_pkt, my_mac, remote_mac, new_ip, r_ip);
>      sendto(_if_sock, ping_pkt, 42, 0,
>             (struct sockaddr *)&iface, sizeof(iface));
>    }
>  
>
Fixed.  It now uses "buffer" as do the BSD and Solaris versions.  That 
change apparently never made it back into the Linux port.

>* wackatrl.c has two close(fd); statements that can be executed -
>  one on successful return from the get_state() function (line 132)
>  and another on exit from the main function (line 198). I would suggest
>  that the first one is removed (from the get_state() function).
>  
>
Indeed.

>* wackatrl.c has faulty command line parsing of the -c option at the
>  top of the main() function:
>
>  char *filename = NULL;
>  ...
>  [Start getopt() loop]
>  case 'c':
>     filename=optarg;
>     break;
>  [Loop around getopt() until done]
>
>  Now, if I run "wackatrl -c config_name -l", then the getopt() loop will
>  finish with filename = NULL (should be filename = config_name). This is
>  because optarg is overwritten each time around the getopt() loop, so the
>  -l option has optarg = NULL and hence so will filename when it exits.
>  The solution is to have a char buffer for filename and snprintf() optarg
>  into it:
>  
>
optarg is changed each loop through, not overwritten.  param = optarg is 
acceptable.  The real problem is that there was no looping!  It was an 
if instead of a while, so after it read -c <filename> it didn't read 
on.  This has been fixed as well.

>I hope these fixes are useful...
>  
>
Always.. Thanks.

-- 
// Theo Schlossnagle
// Principal Engineer -- http://www.omniti.com/~jesus/
// Postal Engine -- http://www.postalengine.com/
// Ecelerity: fastest MTA on Earth