PATCH: netstat '-p' optional feature

Denys Vlasenko vda.linux at googlemail.com
Sun Jul 13 00:13:56 UTC 2008


On Thursday 10 July 2008 23:51, L. Gabriel Somlo wrote:
> Denys & All,
> 
> I needed -p support in busybox's netstat for a project I'm working
> on, so I ported the functionality over from net-tools.
> 
> The attached patch (against svn 22735) adds optional -p support
> allowing netstat to print out process IDs and command names.

I agree that we need to match standard tools. The patch needs a bit of work:


 enum {
 	OPT_extended = 0x4,
 	OPT_showroute = 0x100,
-	OPT_widedisplay = 0x200 * ENABLE_FEATURE_NETSTAT_WIDE,
+	OPT_widedisplay = 0x200,
+	OPT_showprog = 0x400,

Why this change?



+#define PRG_HASH_SIZE 211
+
+static struct prg_node {
+	struct prg_node *next;
+	int inode;
+	char name[PROGNAME_WIDTH];
+} *prg_hash[PRG_HASH_SIZE];
+
+static char prg_cache_loaded = 0;
+
+int flag_prg = 0;

This is a lot of global data. This hurts NOMMU systems.
I suspect flag_prg can be substituted for
(option_mask32 & OPT_showprog)?


+	if (!(*pnp = malloc(sizeof(**pnp)))) 
+		return;

Pull assignment out of if(), please.


+		if (pn->inode == inode) return(pn->name);

busybox uniformly uses return xxx, not return(xxx).


+	if (strlen(lname) < PRG_SOCKET_PFXl + 3 )
+		*inode_p = -1;
+	else if (memcmp(lname, PRG_SOCKET_PFX, PRG_SOCKET_PFXl))
+		*inode_p = -1;
+	else if (lname[strlen(lname) - 1] != ']')
+		*inode_p = -1;
+	else {
+		char inode_str[strlen(lname + 1)];  /* e.g. "12345" */
+		const int inode_str_len = strlen(lname) - PRG_SOCKET_PFXl - 1;
+		char *serr;

Have mercy. Do strlen just once.


+		*inode_p = strtol(inode_str, &serr, 0);
+		if (!serr || *serr || *inode_p < 0 || *inode_p >= INT_MAX) 
+			*inode_p = -1;

bb_strtou is easier to use - you need to check just errno after it.


+	while (errno = 0, direproc = readdir(dirproc)) {

This is even worse than assignment in if().

+		if (direproc->d_type != DT_DIR) continue;

This is not realiable. See the last sentence:

       struct dirent *readdir(DIR *dir);

DESCRIPTION
       The  readdir()  function returns a pointer to a dirent structure repre-
       senting the next directory entry in the directory stream pointed to  by
       dir.   It  returns  NULL  on  reaching  the  end-of-file or if an error
       occurred.

       On Linux, the dirent structure is defined as follows:

          struct dirent {
              ino_t          d_ino;       /* inode number */
              off_t          d_off;       /* offset to the next dirent */
              unsigned short d_reclen;    /* length of this record */
              unsigned char  d_type;      /* type of file */
              char           d_name[256]; /* filename */
          };

       According to POSIX, the dirent structure contains a field char d_name[]
       of  unspecified  size,  with  at most NAME_MAX characters preceding the
       terminating null byte.  POSIX  1003.1-2001  also  documents  the  field
       ino_t  d_ino  as  an  XSI extension.  Use of other fields will harm the
       portability of your programs.


+		for (cs = direproc->d_name; *cs && isdigit(*cs); cs++);

Put dummy "continue" in the loop body, so that everyone will be sure
you intended the loop to be empty.



+			strcpy(line + procfdlen + 1, direfd->d_name);
+			lnamelen=readlink(line, lname, sizeof(lname) - 1);
+            lname[lnamelen] = '\0';  /*make it a null-terminated string*/
+
+            extract_type_1_socket_inode(lname, &inode);
+            if (inode < 0) extract_type_2_socket_inode(lname, &inode);
+            if (inode < 0) continue;

Something went wrong with indentation.


--
vda



More information about the busybox mailing list