[BusyBox 0001130]: arp applet

bugs at busybox.net bugs at busybox.net
Tue Dec 26 04:10:48 UTC 2006


A NOTE has been added to this issue. 
====================================================================== 
http://busybox.net/bugs/view.php?id=1130 
====================================================================== 
Reported By:                espakman
Assigned To:                BusyBox
====================================================================== 
Project:                    BusyBox
Issue ID:                   1130
Category:                   New Features
Reproducibility:            N/A
Severity:                   feature
Priority:                   normal
Status:                     assigned
====================================================================== 
Date Submitted:             12-23-2006 12:35 PST
Last Modified:              12-25-2006 20:10 PST
====================================================================== 
Summary:                    arp applet
Description: 
Busybox currently doesn't contain an arp applet. Someone wrote an arp
applet a long time ago. I rediffed this version against bb 1.3.0 and it
applies cleanly against current SVN.
====================================================================== 

---------------------------------------------------------------------- 
 vda - 12-23-06 23:44  
---------------------------------------------------------------------- 
Unused #define:

+
+#define APPLET_NAME "arp"
+

Inconsistent style: 3-space indent AND 4-space indent in the same file:

+    (void) fclose(fp);
+
+    return 0;
+}
+
+int arp_main(int argc, char **argv)
+{
+   int what, i;
+
+   /* Initialize variables... */
+   if ((hw = get_hwtype(DFLT_HW)) == NULL) {
+      bb_error_msg("%s: hardware type not supported!\n", DFLT_HW);
+      return -1;
+   }

Please reformat to bbox style (indents by 1 tab).

 

---------------------------------------------------------------------- 
 espakman - 12-24-06 09:07  
---------------------------------------------------------------------- 
Second try, new patch attached. 

---------------------------------------------------------------------- 
 espakman - 12-24-06 10:42  
---------------------------------------------------------------------- 
Now run through indent, to be fully bbox style complient. 

---------------------------------------------------------------------- 
 vda - 12-25-06 20:10  
---------------------------------------------------------------------- 
Reviewed the patch.

+/* This structure defines protocol families and their handlers. */
+struct aftype {
+        char *name;
+        char *title;
+        int af;
+        int alen;

Use tabs for indentation.

+#define arp_trivial_usage \
+        "[-evn] [-H type] [-i if] -a [hostname]\n" \
+        "\tarp [-v] [-i if] -d hostname [pub]\n" \
+        "\tarp [-v] [-H type] [-i if] -s hostname hw_addr [temp]\n" \

Follow usage.h rules which are written in the comment on top of that
file.

+#include <sys/ioctl.h>
+#include <sys/signal.h>
+#include <sys/time.h>
+
+#include <errno.h>
+#include <netdb.h>
+#include <stdlib.h>
+#include <string.h>
+#include <ctype.h>
+#include <unistd.h>
+
+#include <arpa/inet.h>
+#include <net/if.h>
+#include <net/if_arp.h>
+#include <netinet/ether.h>
+#include <netpacket/packet.h>
+
+#include "busybox.h"

Most of these standard includes are already included by #include
"busybox.h"

+extern struct aftype *get_aftype(const char *name);
+extern struct hwtype *get_hwntype(int type);
+extern struct hwtype *get_hwtype(const char *name);
+
+int opt_n = 0;
+int opt_v = 0;

These functions and variables should be static.

+               /* Skip trailing whitespace. */
+               if (*ptr != '\0') {
+                       while (*ptr == ' ' || *ptr == '\t')
+                               ptr++;
+               }

We have skip_whitespace, use that.

+       memcpy((char *) &req.arp_pa, (char *) &sa, sizeof(struct
sockaddr));

Casts are superfluous here.

+                       bb_error_msg("args=%s\n", *args);

bb_[ph]error_msgXXX print trailing '\n' by themself.

+               perror("SIOCSARP");

use bb_perror_msg

+                       ("Address                  HWtype  HWaddress      
    Flags Mask            Iface\n");

Maybe use tabs?

+static int arp_show(char *name)
+{
+       char host[100];
+       struct sockaddr sa;
+       char ip[100];
+       char hwa[100];
+       char mask[100];
+       char line[200];
+       char dev[100];
+       int type, flags;
+       FILE *fp;
+       char *hostname;
+       int num, entries = 0, showed = 0;
+
....
+       /* Bypass header -- read until newline */
+       if (fgets(line, sizeof(line), fp) != (char *) NULL) {

use xmalloc_fget{s,line} instead of fixed-sized buffers. It saves memory.

+       (void) fclose(fp);

Don't think we need this cast

+                               bb_error_msg("%s: unknown address
family.\n", optarg);

Trailing period (and '\n') is not needed 

Issue History 
Date Modified   Username       Field                    Change               
====================================================================== 
12-23-06 12:35  espakman       New Issue                                    
12-23-06 12:35  espakman       Status                   new => assigned     
12-23-06 12:35  espakman       Assigned To               => BusyBox         
12-23-06 12:35  espakman       File Added: busybox-arp.patch                    
12-23-06 23:44  vda            Note Added: 0001931                          
12-23-06 23:44  vda            Note Edited: 0001931                         
12-24-06 09:06  espakman       File Added: busybox-arp2.patch                   

12-24-06 09:07  espakman       Note Added: 0001932                          
12-24-06 10:41  espakman       File Added: busybox-arp3.patch                   

12-24-06 10:42  espakman       Note Added: 0001933                          
12-25-06 20:10  vda            Note Added: 0001934                          
======================================================================




More information about the busybox-cvs mailing list