[BusyBox 0001343]: arp ethers support

[bugs at busybox.net]

A NOTE has been added to this issue. 
====================================================================== 
http://busybox.net/bugs/view.php?id=1343 
====================================================================== 
Reported By:                ralf
Assigned To:                BusyBox
====================================================================== 
Project:                    BusyBox
Issue ID:                   1343
Category:                   Networking Support
Reproducibility:            always
Severity:                   feature
Priority:                   normal
Status:                     assigned
====================================================================== 
Date Submitted:             05-11-2007 07:22 PDT
Last Modified:              05-26-2007 21:17 PDT
====================================================================== 
Summary:                    arp ethers support
Description: 
With this patch arp can read addresses from a static file
====================================================================== 

---------------------------------------------------------------------- 
 vda - 05-20-07 06:29  
---------------------------------------------------------------------- 
Indeed, "standard" arp has this option. However, I can't apply this patch
as-is:

1. -f option is not made conditional (it should be, because this
functionality is not essential for every user).

2. If I understand it right, /etc/ethers looks like:
00:00:4b:3e:45:e2 192.168.1.1
00:00:56:3e:36:1f 192.168.1.2
Your parsing code seems to be too complex for such a simple file format.
If you handle more complex format, please document example(s) in source
code comments.

3. arp_file() body is whitespace damaged; and please avoid assignments in
ifs like 'if ((fp = fopen(name, "r"))' 

---------------------------------------------------------------------- 
 ralf - 05-20-07 10:35  
---------------------------------------------------------------------- 
vda: i was using an older busybox, now i have the current svn patch

1. True, even if the amount of code is so little...
2. yes, the syntax is like: 00:00:4b:3e:45:e2 192.168.1.1 but wh have to
handle "" and \ as the ip could be an hostname. btw, the code was taken
from standard net-tools
3: fixed 

---------------------------------------------------------------------- 
 ralf - 05-20-07 10:40  
---------------------------------------------------------------------- 
sorry, removed 'int arp_main(int argc, char **argv);' by mistake, fixed in
ethers3.diff
Could someone delete older patches? tnx 

---------------------------------------------------------------------- 
 vda - 05-26-07 21:17  
---------------------------------------------------------------------- 
Sorry. Still far, far too big. Just one example:

+               if (*ptr != '\0') {
+                       while (*ptr == ' ' || *ptr == '\t')
+                               ptr++;
+               }

Do you see that if() is completetely useless here and can be removed,
leaving only while()?

My suggestion:

+static int arp_file(char *name)
+{
+       char buff[1024];
+       char *sp, *args[32];
+       int linenr, argc;
+       FILE *fp = fopen(name, "r");
+       if(!fp) {
+               bb_error_msg_and_die("arp: cannot open etherfile %s !",
name);
+               return (-1);
+       }

Use xfopen.

+       /* Read the lines in the file. */
+       linenr = 0;
+       while(fgets(buff, sizeof(buff), fp) != (char *)NULL) {

Don't use big fixed buffer. Use xmalloc_getline instead.
(Will nicely eliminate the need in "if((sp = strchr(buff, '\n'))..." too)

+               argc = getargs(buff, args);
+               if(argc < 2) {
+                       bb_error_msg("arp: format error on line %u of
etherfile %s !", linenr, name);
+                       continue;
+               }

Completely rewrite getargs(), making it die (with msg) on errors, and
handling only [<whitespace>]ether<whitespace>IP_or_hostname[<whitespace>]
syntax. This should be simple - and SMALL.

Thus here you won't need argc. You will *know* that there are two
arguments.

> btw, the code was taken from standard net-tools

This cannot be used as justification for inclusion. Sorry. 

Issue History 
Date Modified   Username       Field                    Change               
====================================================================== 
05-11-07 07:22  ralf           New Issue                                    
05-11-07 07:22  ralf           Status                   new => assigned     
05-11-07 07:22  ralf           Assigned To               => BusyBox         
05-11-07 07:22  ralf           File Added: ethers.diff                      
05-11-07 07:26  ralf           Issue Monitored: ralf                        
05-20-07 06:29  vda            Note Added: 0002384                          
05-20-07 10:35  ralf           Note Added: 0002386                          
05-20-07 10:36  ralf           File Added: ethers2.diff                     
05-20-07 10:39  ralf           File Added: ethers3.diff                     
05-20-07 10:40  ralf           Note Added: 0002387                          
05-26-07 21:17  vda            Note Added: 0002405                          
======================================================================