[PATCH v2 0/2] iptraf-ng: get the interface name only when needed

[Vitezslav Samel]

Right now on every received packet we get the interface name
on which it arrived. This is not needed and is slow.
Make use of ifindex and retrieve interface name only when needed
(when capturing on "All interfaces").

v2:
  - hostmon(): bugfix: ifname --> ifnamebuf
  - get rid of passing buffer space (must be always at least IFNAMSIZ)
  - C style fixes
  - always check for errors from system/library calls (socket(), ioctl(), ...)
v1:
  - initial version

Vitezslav Samel (2):
  get the interface name only when needed
  remove unused params from getpacket() and processpacket()

 src/hostmon.c  |   20 +++++++++++++--
 src/ifaces.c   |   68 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/ifaces.h   |    2 +
 src/ifstats.c  |   71 ++++++++++++++++++++++++++++++-------------------------
 src/ifstats.h  |    1 +
 src/itrafmon.c |   20 +++++++++++++--
 src/packet.c   |   16 +-----------
 src/packet.h   |    4 +-
 src/pktsize.c  |    6 +---
 src/serv.c     |    5 +--
 10 files changed, 152 insertions(+), 61 deletions(-)

-- 
1.7.8.4

[PATCH 1/2] get the interface name only when needed

[Vitezslav Samel]

Right now on every received packet we get the interface name
on which it arrived. This is not needed and is slow.
Make use of ifindex and retrieve interface name only when needed
(when capturing on "All interfaces").

Signed-off-by: Vitezslav Samel <vitezslav(a)samel.cz&gt;
---
 src/hostmon.c  |   20 +++++++++++++--
 src/ifaces.c   |   68 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/ifaces.h   |    2 +
 src/ifstats.c  |   71 ++++++++++++++++++++++++++++++-------------------------
 src/ifstats.h  |    1 +
 src/itrafmon.c |   20 +++++++++++++--
 src/packet.c   |    8 ++++--
 src/pktsize.c  |    5 +--
 src/serv.c     |    5 +--
 9 files changed, 153 insertions(+), 47 deletions(-)

diff --git a/src/hostmon.c b/src/hostmon.c
index 32e1f1a..5f3dd8a 100644
--- a/src/hostmon.c
+++ b/src/hostmon.c
@@ -678,7 +678,7 @@ void hostmon(const struct OPTIONS *options, int facilitytime, char
*ifptr,
 	int is_ip;
 	int ch;
 
-	char ifname[IFNAMSIZ];
+	char *ifname = ifptr;
 
 	struct timeval tv;
 	unsigned long starttime;
@@ -814,7 +814,7 @@ void hostmon(const struct OPTIONS *options, int facilitytime, char
*ifptr,
 		    && (((now - statbegin) / 60) >= facilitytime))
 			exitloop = 1;
 
-		getpacket(fd, buf, &fromaddr, &ch, &br, ifname, table.tabwin);
+		getpacket(fd, buf, &fromaddr, &ch, &br, NULL, table.tabwin);
 
 		if (ch != ERR) {
 			if (keymode == 0) {
@@ -860,16 +860,30 @@ void hostmon(const struct OPTIONS *options, int facilitytime, char
*ifptr,
 			}
 		}
 		if (br > 0) {
+			char ifnamebuf[IFNAMSIZ];
+
 			pkt_result =
 			    processpacket(buf, &ipacket, (unsigned int *) &br,
 					  NULL, NULL, NULL, &fromaddr,
 					  ofilter,
-					  MATCH_OPPOSITE_USECONFIG, ifname,
+					  MATCH_OPPOSITE_USECONFIG, NULL,
 					  0);
 
 			if (pkt_result != PACKET_OK)
 				continue;
 
+			if (!ifptr) {
+				/* we're capturing on "All interfaces", */
+				/* so get the name of the interface */
+				/* of this packet */
+				int r = iface_get_ifname(fromaddr.sll_ifindex, ifnamebuf);
+				if (r != 0) {
+					write_error("Unable to get interface name");
+					break;	/* can't get interface name, get out! */
+				}
+				ifname = ifnamebuf;
+			}
+
 			if ((fromaddr.sll_hatype == ARPHRD_ETHER)
 			    || (fromaddr.sll_hatype == ARPHRD_FDDI)
 /* fix me
diff --git a/src/ifaces.c b/src/ifaces.c
index c1e0be5..81feca9 100644
--- a/src/ifaces.c
+++ b/src/ifaces.c
@@ -100,6 +100,74 @@ void err_iface_down(void)
 	write_error("Specified interface not active");
 }
 
+int iface_get_ifindex(const char *iface)
+{
+	int fd;
+	int ir;
+	struct ifreq ifr;
+	int saved_errno;
+	int cl;
+
+	fd = socket(PF_INET, SOCK_DGRAM, IPPROTO_UDP);
+	if (fd == -1)
+		return fd;
+
+	strcpy(ifr.ifr_name, iface);
+	ir = ioctl(fd, SIOCGIFINDEX, &ifr);
+
+	/* need to preserve errno across call to close() */
+	saved_errno = errno;
+
+	cl = close(fd);
+
+	/* bug out if ioctl() failed */
+	if (ir != 0) {
+		errno = saved_errno;
+		return ir;
+	}
+
+	/* bug out if close() failed */
+	if (cl != 0)
+		return cl;
+
+	return ifr.ifr_ifindex;
+}
+
+int iface_get_ifname(int ifindex, char *ifname)
+{
+	int fd;
+	int ir;
+	struct ifreq ifr = {
+		.ifr_ifindex = ifindex
+	};
+	int saved_errno;
+	int cl;
+
+	fd = socket(PF_INET, SOCK_DGRAM, IPPROTO_UDP);
+	if (fd == -1)
+		return fd;
+
+	ir = ioctl(fd, SIOCGIFNAME, &ifr);
+
+	/* need to preserve errno across call to close() */
+	saved_errno = errno;
+
+	cl = close(fd);
+
+	/* bug out if ioctl() failed */
+	if (ir != 0) {
+		errno = saved_errno;
+		return ir;
+	}
+
+	/* bug out if close() failed */
+	if (cl != 0)
+		return cl;
+
+	strncpy(ifname, ifr.ifr_name, IFNAMSIZ);
+	return ir;
+}
+
 void isdn_iface_check(int *fd, char *ifname)
 {
 	if (*fd == -1) {
diff --git a/src/ifaces.h b/src/ifaces.h
index 5bf313d..fa20324 100644
--- a/src/ifaces.h
+++ b/src/ifaces.h
@@ -11,6 +11,8 @@ ifaces.h - prototype declaration for interface support determination
 FILE *open_procnetdev(void);
 int get_next_iface(FILE * fd, char *ifname, int n);
 int iface_up(char *iface);
+int iface_get_ifindex(const char *iface);
+int iface_get_ifname(int ifindex, char *ifname);
 void err_iface_down(void);
 void isdn_iface_check(int *fd, char *ifname);
 char *gen_iface_msg(char *ifptr);
diff --git a/src/ifstats.c b/src/ifstats.c
index d34db9e..3c06083 100644
--- a/src/ifstats.c
+++ b/src/ifstats.c
@@ -115,6 +115,8 @@ void initiflist(struct iflist **list)
 	}
 
 	while (get_next_iface(fd, ifname, sizeof(ifname))) {
+		int ifindex;
+
 		if (!*ifname)
 			continue;
 
@@ -130,6 +132,9 @@ void initiflist(struct iflist **list)
 		if (!iface_up(ifname))
 			continue;
 
+		ifindex = iface_get_ifindex(ifname);
+		if (ifindex < 0)
+			continue;
 		/*
 		 * At this point, the interface is now sure to be up and running.
 		 */
@@ -137,6 +142,7 @@ void initiflist(struct iflist **list)
 		itmp = xmalloc(sizeof(struct iflist));
 		memset(itmp, 0, sizeof(struct iflist));
 		strcpy(itmp->ifname, ifname);
+		itmp->ifindex = ifindex;
 		index++;
 		itmp->index = index;
 
@@ -155,36 +161,36 @@ void initiflist(struct iflist **list)
 	fclose(fd);
 }
 
-void positionptr(struct iftab *table, struct iflist **ptmp, char *ifname)
+struct iflist *positionptr(struct iflist *iflist, const int ifindex)
 {
-	struct iflist *plast = NULL;
-	int ok = 0;
-
-	*ptmp = table->head;
+	struct iflist *ptmp = iflist;
+	struct iflist *last = ptmp;
 
-	while ((*ptmp != NULL) && (!ok)) {
-		ok = (strcmp((*ptmp)->ifname, ifname) == 0);
-
-		if (!ok) {
-			if ((*ptmp)->next_entry == NULL)
-				plast = *ptmp;
-
-			*ptmp = (*ptmp)->next_entry;
-		}
+	while ((ptmp != NULL) && (ptmp->ifindex != ifindex)) {
+		last = ptmp;
+		ptmp = ptmp->next_entry;
 	}
+	/* no interface was found, try to create new one */
+	if (ptmp == NULL) {
+		struct iflist *itmp;
+		int r;
+
+		itmp = xmallocz(sizeof(struct iflist));
+		itmp->ifindex = ifindex;
+		itmp->index = last->index + 1;
+		r = iface_get_ifname(ifindex, itmp->ifname);
+		if (r != 0) {
+			write_error("Error getting interface name");
+			return(NULL);
+		}
 
-	if (*ptmp == NULL) {
-		*ptmp = xmalloc(sizeof(struct iflist));
-		memset(*ptmp, 0, sizeof(struct iflist));
-		(*ptmp)->index = plast->index + 1;
-		plast->next_entry = *ptmp;
-		(*ptmp)->prev_entry = plast;
-		(*ptmp)->next_entry = NULL;
-		strcpy((*ptmp)->ifname, ifname);
-
-		if ((*ptmp)->index <= LINES - 4)
-			table->lastvisible = *ptmp;
+		/* last can't be NULL otherwise we will have empty iflist */
+		last->next_entry = itmp;
+		itmp->prev_entry = last;
+		itmp->next_entry = NULL;
+		ptmp = itmp;
 	}
+	return(ptmp);
 }
 
 void destroyiflist(struct iflist *list)
@@ -406,7 +412,6 @@ void ifstats(const struct OPTIONS *options, struct filterstate
*ofilter,
 	FILE *logfile = NULL;
 
 	int br;
-	char ifname[IFNAMSIZ];
 
 	int ch;
 
@@ -520,7 +525,7 @@ void ifstats(const struct OPTIONS *options, struct filterstate
*ofilter,
 		    && (((now - statbegin) / 60) >= facilitytime))
 			exitloop = 1;
 
-		getpacket(fd, buf, &fromaddr, &ch, &br, ifname, table.statwin);
+		getpacket(fd, buf, &fromaddr, &ch, &br, NULL, table.statwin);
 
 		switch (ch) {
 		case ERR:
@@ -561,13 +566,15 @@ void ifstats(const struct OPTIONS *options, struct filterstate
*ofilter,
 					   NULL, NULL, NULL, &fromaddr,
 					   ofilter,
 					   MATCH_OPPOSITE_USECONFIG,
-					   ifname, options->v6inv4asv6);
+					   NULL, options->v6inv4asv6);
 
 		if (pkt_result != PACKET_OK
 		    && pkt_result != MORE_FRAGMENTS)
 			continue;
 
-		positionptr(&table, &ptmp, ifname);
+		ptmp = positionptr(table.head, fromaddr.sll_ifindex);
+		if (!ptmp)
+			continue;
 
 		ptmp->total++;
 
@@ -743,7 +750,6 @@ void detstats(char *iface, const struct OPTIONS *options, int
facilitytime,
 	struct iphdr *ipacket = NULL;
 	struct ip6_hdr *ip6packet = NULL;
 
-	char ifname[IFNAMSIZ];
 	struct sockaddr_ll fromaddr;
 
 	int br;
@@ -981,7 +987,7 @@ void detstats(char *iface, const struct OPTIONS *options, int
facilitytime,
 		    && (((now - statbegin) / 60) >= facilitytime))
 			exitloop = 1;
 
-		getpacket(fd, buf, &fromaddr, &ch, &br, ifname, statwin);
+		getpacket(fd, buf, &fromaddr, &ch, &br, NULL, statwin);
 
 		switch (ch) {
 		case ERR:
@@ -1011,7 +1017,7 @@ void detstats(char *iface, const struct OPTIONS *options, int
facilitytime,
 			    processpacket(buf, &packet, (unsigned int *) &br,
 					  NULL, NULL, NULL, &fromaddr,
 					  ofilter,
-					  MATCH_OPPOSITE_USECONFIG, ifname,
+					  MATCH_OPPOSITE_USECONFIG, NULL,
 					  options->v6inv4asv6);
 
 			if (pkt_result != PACKET_OK
@@ -1124,6 +1130,7 @@ void selectiface(char *ifname, int withall, int *aborted)
 	if ((withall) && (list != NULL)) {
 		ptmp = xmalloc(sizeof(struct iflist));
 		strncpy(ptmp->ifname, "All interfaces", sizeof(ptmp->ifname));
+		ptmp->ifindex = 0;
 
 		ptmp->prev_entry = NULL;
 		list->prev_entry = ptmp;
diff --git a/src/ifstats.h b/src/ifstats.h
index 781c8df..1dfb517 100644
--- a/src/ifstats.h
+++ b/src/ifstats.h
@@ -9,6 +9,7 @@ ifstats.h - structure definitions for interface counts
 
 struct iflist {
 	char ifname[IFNAMSIZ];
+	int ifindex;
 	unsigned int encap;
 	unsigned long long iptotal;
 	unsigned long long ip6total;
diff --git a/src/itrafmon.c b/src/itrafmon.c
index dea71b2..c32c19d 100644
--- a/src/itrafmon.c
+++ b/src/itrafmon.c
@@ -602,7 +602,7 @@ void ipmon(struct OPTIONS *options, struct filterstate *ofilter,
 	int curwin = 0;
 
 	int readlen;
-	char ifname[IFNAMSIZ];
+	char *ifname = ifptr;
 
 	unsigned long long total_pkts = 0;
 
@@ -758,6 +758,8 @@ void ipmon(struct OPTIONS *options, struct filterstate *ofilter,
 	starttime = timeint = closedint = tv.tv_sec;
 
 	while (!exitloop) {
+		char ifnamebuf[IFNAMSIZ];
+
 		gettimeofday(&tv, NULL);
 		now = tv.tv_sec;
 		unow = tv.tv_sec * 1e+06 + tv.tv_usec;
@@ -833,7 +835,7 @@ void ipmon(struct OPTIONS *options, struct filterstate *ofilter,
 			rotate_flag = 0;
 		}
 
-		getpacket(fd, tpacket, &fromaddr, &ch, &readlen, ifname,
+		getpacket(fd, tpacket, &fromaddr, &ch, &readlen, NULL,
 			  table.tcpscreen);
 
 		if (ch == ERR)
@@ -1042,11 +1044,23 @@ void ipmon(struct OPTIONS *options, struct filterstate *ofilter,
 				  (unsigned int *) &readlen, &br,
 				  &sport, &dport, &fromaddr,
 				  ofilter, MATCH_OPPOSITE_ALWAYS,
-				  ifname, options->v6inv4asv6);
+				  NULL, options->v6inv4asv6);
 
 		if (pkt_result != PACKET_OK)
 			continue;
 
+		if (!ifptr) {
+			/* we're capturing on "All interfaces", */
+			/* so get the name of the interface */
+			/* of this packet */
+			int r = iface_get_ifname(fromaddr.sll_ifindex, ifnamebuf);
+			if (r != 0) {
+				write_error("Unable to get interface name");
+				break;          /* error getting interface name, get out! */
+			}
+			ifname = ifnamebuf;
+		}
+
 		switch(fromaddr.sll_protocol) {
 		case ETH_P_IP:
 			ippacket = (struct iphdr *) packet;
diff --git a/src/packet.c b/src/packet.c
index c012d17..0753ee0 100644
--- a/src/packet.c
+++ b/src/packet.c
@@ -160,9 +160,11 @@ void getpacket(int fd, char *buf, struct sockaddr_ll *fromaddr, int
*ch,
 		fromlen = sizeof(struct sockaddr_ll);
 		*br = recvfrom(fd, buf, MAX_PACKET_SIZE, 0,
 			       (struct sockaddr *) fromaddr, &fromlen);
-		ifr.ifr_ifindex = fromaddr->sll_ifindex;
-		ioctl(fd, SIOCGIFNAME, &ifr);
-		strcpy(ifname, ifr.ifr_name);
+		if (ifname) {
+			ifr.ifr_ifindex = fromaddr->sll_ifindex;
+			ioctl(fd, SIOCGIFNAME, &ifr);
+			strcpy(ifname, ifr.ifr_name);
+		}
 	}
 	if (!daemonized && FD_ISSET(0, &set))
 		*ch = wgetch(win);
diff --git a/src/pktsize.c b/src/pktsize.c
index 5bfbeaf..ce0fd78 100644
--- a/src/pktsize.c
+++ b/src/pktsize.c
@@ -155,7 +155,6 @@ void packet_size_breakdown(struct OPTIONS *options, char *ifname,
 	char buf[MAX_PACKET_SIZE];
 	int br;
 	char *ipacket;
-	char iface[IFNAMSIZ];
 	unsigned int mtu;
 
 	struct sockaddr_ll fromaddr;
@@ -294,7 +293,7 @@ void packet_size_breakdown(struct OPTIONS *options, char *ifname,
 		    && (((now - starttime) / 60) >= facilitytime))
 			exitloop = 1;
 
-		getpacket(fd, buf, &fromaddr, &ch, &br, iface, win);
+		getpacket(fd, buf, &fromaddr, &ch, &br, NULL, win);
 
 		if (ch != ERR) {
 			switch (ch) {
@@ -317,7 +316,7 @@ void packet_size_breakdown(struct OPTIONS *options, char *ifname,
 			    processpacket(buf, &ipacket, (unsigned int *) &br,
 					  NULL, NULL, NULL, &fromaddr,
 					  ofilter,
-					  MATCH_OPPOSITE_USECONFIG, iface,
+					  MATCH_OPPOSITE_USECONFIG, NULL,
 					  0);
 
 			if (pkt_result != PACKET_OK)
diff --git a/src/serv.c b/src/serv.c
index 74a3c6c..f0cd39f 100644
--- a/src/serv.c
+++ b/src/serv.c
@@ -699,7 +699,6 @@ void servmon(char *ifname, struct porttab *ports, const struct OPTIONS
*options,
 	struct sockaddr_ll fromaddr;
 	int br;
 
-	char iface[IFNAMSIZ];
 	unsigned int idx = 1;
 
 	unsigned int sport = 0;
@@ -863,7 +862,7 @@ void servmon(char *ifname, struct porttab *ports, const struct OPTIONS
*options,
 		    && (((now - starttime) / 60) >= facilitytime))
 			exitloop = 1;
 
-		getpacket(fd, buf, &fromaddr, &ch, &br, iface, list.win);
+		getpacket(fd, buf, &fromaddr, &ch, &br, NULL, list.win);
 
 		if (ch == ERR)
 			goto no_key_ready;
@@ -998,7 +997,7 @@ void servmon(char *ifname, struct porttab *ports, const struct OPTIONS
*options,
 			processpacket(buf, &ipacket, (unsigned int *) &br,
 				      &tot_br, &sport, &dport, &fromaddr,
 				      ofilter,
-				      MATCH_OPPOSITE_USECONFIG, iface,
+				      MATCH_OPPOSITE_USECONFIG, NULL,
 				      options->v6inv4asv6);
 
 		if (pkt_result != PACKET_OK)
-- 
1.7.8.4

[PATCH 1/2] get the interface name only when needed

[Nikola Pajkovsky]

Vitezslav Samel <vitezslav(a)samel.cz&gt; writes:

Horrible, horrible c89 is! Please use only c99/gnu99 standard.

>  +int iface_get_ifname(int ifindex, char *ifname)
>  +{
>  +	int fd;
>  +	int ir;
>  +	struct ifreq ifr = {
>  +		.ifr_ifindex = ifindex
>  +	};
>  +	int saved_errno;
>  +	int cl;
>  +
>  +	fd = socket(PF_INET, SOCK_DGRAM, IPPROTO_UDP);
>  +	if (fd == -1)
>  +		return fd;
>  +
>  +	ir = ioctl(fd, SIOCGIFNAME, &ifr);
>  +
>  +	/* need to preserve errno across call to close() */
>  +	saved_errno = errno;
>  +
>  +	cl = close(fd);
>  +
>  +	/* bug out if ioctl() failed */
>  +	if (ir != 0) {
>  +		errno = saved_errno;
>  +		return ir;
>  +	}
>  +
>  +	/* bug out if close() failed */
>  +	if (cl != 0)
>  +		return cl;
>  +
>  +	strncpy(ifname, ifr.ifr_name, IFNAMSIZ);
>  +	return ir;
>  +}
>  + 
We dont have to check close() here. According man:

"Not  checking the return value of *close()* is a common but nevertheless
serious programming error. It is quite possible that errors on a
previous *write(2)* operatio n are first reported at the final *close()*.
Not checking the return value when closing the file may lead to silent
loss of data.  This can especially be observed with NFS and with disk
quota."

Thus, when you write, check *close()*, when you read, you don't have
to. Nevertheless we are reading, so don't checking.

If we won't check errno and print some common message, than we don't
have to do the machinery about saving and restoring errno.

I'd suggest shorter version of func like that:

+int dev_ifname(int ifindex, char *ifname)
+{
+       int fd = socket(PF_INET, SOCK_DGRAM, IPPROTO_UDP);
+       if (fd == -1) // or < 0, I don't care
+               return fd;
+
+       struct ifreq ifr = {
+               .ifr_ifindex = ifindex
+       };
+
+       int ir = ioctl(fd, SIOCGIFNAME, &ifr);
+       close(fd);
+       if (ir)
+               return ir;
+
+       strncpy(ifname, ifr.ifr_name, IFNAMSIZ);
+       return ir;
+}

just for information SIOCGIFNAME cmd returns on error -EFAULT and
-ENODEV, on success 0 is returned.


otherwise patch is good ;)

-- 
Nikola

[PATCH 1/2] get the interface name only when needed

[Vitezslav Samel]

On Tue, Apr 10, 2012 at 12:37:58PM +0200, Nikola Pajkovsky wrote:
>  Vitezslav Samel <vitezslav(a)samel.cz&gt; writes:
>  
>  Horrible, horrible c89 is! Please use only c99/gnu99 standard. 
  I will subordinate to this ;-)))

>  > +int iface_get_ifname(int ifindex, char *ifname)
>  > +{
>  > +	int fd;
>  > +	int ir;
>  > +	struct ifreq ifr = {
>  > +		.ifr_ifindex = ifindex
>  > +	};
>  > +	int saved_errno;
>  > +	int cl;
>  > +
>  > +	fd = socket(PF_INET, SOCK_DGRAM, IPPROTO_UDP);
>  > +	if (fd == -1)
>  > +		return fd;
>  > +
>  > +	ir = ioctl(fd, SIOCGIFNAME, &ifr);
>  > +
>  > +	/* need to preserve errno across call to close() */
>  > +	saved_errno = errno;
>  > +
>  > +	cl = close(fd);
>  > +
>  > +	/* bug out if ioctl() failed */
>  > +	if (ir != 0) {
>  > +		errno = saved_errno;
>  > +		return ir;
>  > +	}
>  > +
>  > +	/* bug out if close() failed */
>  > +	if (cl != 0)
>  > +		return cl;
>  > +
>  > +	strncpy(ifname, ifr.ifr_name, IFNAMSIZ);
>  > +	return ir;
>  > +}
>  > +
>  
>  We dont have to check close() here. According man:
>  
>  "Not  checking the return value of *close()* is a common but nevertheless
>  serious programming error. It is quite possible that errors on a
>  previous *write(2)* operation are first reported at the final *close()*.
>  Not checking the return value when closing the file may lead to silent
>  loss of data.  This can especially be observed with NFS and with disk
>  quota." 
  Yes, I read it.

>  Thus, when you write, check *close()*, when you read, you don't
> have
>  to. Nevertheless we are reading, so don't checking. 
  OK, won't check.

>  If we won't check errno and print some common message, than we
> don't
>  have to do the machinery about saving and restoring errno. 
  No, I will leave this my way (with errno saving) to prevent funny
surprises when we'll learn how to print error string.

	Vita

[PATCH 1/2] get the interface name only when needed

[Nikola Pajkovsky]

Vitezslav Samel <vitezslav(a)samel.cz&gt; writes:

>  On Tue, Apr 10, 2012 at 12:37:58PM +0200, Nikola Pajkovsky wrote:
> > Vitezslav Samel <vitezslav(a)samel.cz&gt; writes:
> > 
> > Horrible, horrible c89 is! Please use only c99/gnu99 standard.
>
>    I will subordinate to this ;-))) 
hehe, it's better ;)

> > > +int iface_get_ifname(int ifindex, char *ifname)
> > > +{
> > > +	int fd;
> > > +	int ir;
> > > +	struct ifreq ifr = {
> > > +		.ifr_ifindex = ifindex
> > > +	};
> > > +	int saved_errno;
> > > +	int cl;
> > > +
> > > +	fd = socket(PF_INET, SOCK_DGRAM, IPPROTO_UDP);
> > > +	if (fd == -1)
> > > +		return fd;
> > > +
> > > +	ir = ioctl(fd, SIOCGIFNAME, &ifr);
> > > +
> > > +	/* need to preserve errno across call to close() */
> > > +	saved_errno = errno;
> > > +
> > > +	cl = close(fd);
> > > +
> > > +	/* bug out if ioctl() failed */
> > > +	if (ir != 0) {
> > > +		errno = saved_errno;
> > > +		return ir;
> > > +	}
> > > +
> > > +	/* bug out if close() failed */
> > > +	if (cl != 0)
> > > +		return cl;
> > > +
> > > +	strncpy(ifname, ifr.ifr_name, IFNAMSIZ);
> > > +	return ir;
> > > +}
> > > +
> > 
> > We dont have to check close() here. According man:
> > 
> > "Not  checking the return value of *close()* is a common but nevertheless
> > serious programming error. It is quite possible that errors on a
> > previous *write(2)* operation are first reported at the final *close()*.
> > Not checking the return value when closing the file may lead to silent
> > loss of data.  This can especially be observed with NFS and with disk
> > quota."
>
>    Yes, I read it.
>
> > Thus, when you write, check *close()*, when you read, you don't have
> > to. Nevertheless we are reading, so don't checking.
>
>    OK, won't check.
>
> > If we won't check errno and print some common message, than we don't
> > have to do the machinery about saving and restoring errno.
>
>    No, I will leave this my way (with errno saving) to prevent funny
>  surprises when we'll learn how to print error string. 
ok, but I doubt that ever happens ;)

-- 
Nikola

[PATCH 2/2] remove unused params from getpacket() and processpacket()

[Vitezslav Samel]

Since all the callers have ifname NULL we can remove the params
and associated code.
In processpacket() I deleted also ifdefed code which checks for ISDN
interface (this needs to be done in other way).

Signed-off-by: Vitezslav Samel <vitezslav(a)samel.cz&gt;
---
 src/hostmon.c  |    4 ++--
 src/ifstats.c  |    8 ++++----
 src/itrafmon.c |    4 ++--
 src/packet.c   |   18 ++----------------
 src/packet.h   |    4 ++--
 src/pktsize.c  |    5 ++---
 src/serv.c     |    4 ++--
 7 files changed, 16 insertions(+), 31 deletions(-)

diff --git a/src/hostmon.c b/src/hostmon.c
index 5f3dd8a..03806de 100644
--- a/src/hostmon.c
+++ b/src/hostmon.c
@@ -814,7 +814,7 @@ void hostmon(const struct OPTIONS *options, int facilitytime, char
*ifptr,
 		    && (((now - statbegin) / 60) >= facilitytime))
 			exitloop = 1;
 
-		getpacket(fd, buf, &fromaddr, &ch, &br, NULL, table.tabwin);
+		getpacket(fd, buf, &fromaddr, &ch, &br, table.tabwin);
 
 		if (ch != ERR) {
 			if (keymode == 0) {
@@ -866,7 +866,7 @@ void hostmon(const struct OPTIONS *options, int facilitytime, char
*ifptr,
 			    processpacket(buf, &ipacket, (unsigned int *) &br,
 					  NULL, NULL, NULL, &fromaddr,
 					  ofilter,
-					  MATCH_OPPOSITE_USECONFIG, NULL,
+					  MATCH_OPPOSITE_USECONFIG,
 					  0);
 
 			if (pkt_result != PACKET_OK)
diff --git a/src/ifstats.c b/src/ifstats.c
index 3c06083..bf1c6f8 100644
--- a/src/ifstats.c
+++ b/src/ifstats.c
@@ -525,7 +525,7 @@ void ifstats(const struct OPTIONS *options, struct filterstate
*ofilter,
 		    && (((now - statbegin) / 60) >= facilitytime))
 			exitloop = 1;
 
-		getpacket(fd, buf, &fromaddr, &ch, &br, NULL, table.statwin);
+		getpacket(fd, buf, &fromaddr, &ch, &br, table.statwin);
 
 		switch (ch) {
 		case ERR:
@@ -566,7 +566,7 @@ void ifstats(const struct OPTIONS *options, struct filterstate
*ofilter,
 					   NULL, NULL, NULL, &fromaddr,
 					   ofilter,
 					   MATCH_OPPOSITE_USECONFIG,
-					   NULL, options->v6inv4asv6);
+					   options->v6inv4asv6);
 
 		if (pkt_result != PACKET_OK
 		    && pkt_result != MORE_FRAGMENTS)
@@ -987,7 +987,7 @@ void detstats(char *iface, const struct OPTIONS *options, int
facilitytime,
 		    && (((now - statbegin) / 60) >= facilitytime))
 			exitloop = 1;
 
-		getpacket(fd, buf, &fromaddr, &ch, &br, NULL, statwin);
+		getpacket(fd, buf, &fromaddr, &ch, &br, statwin);
 
 		switch (ch) {
 		case ERR:
@@ -1017,7 +1017,7 @@ void detstats(char *iface, const struct OPTIONS *options, int
facilitytime,
 			    processpacket(buf, &packet, (unsigned int *) &br,
 					  NULL, NULL, NULL, &fromaddr,
 					  ofilter,
-					  MATCH_OPPOSITE_USECONFIG, NULL,
+					  MATCH_OPPOSITE_USECONFIG,
 					  options->v6inv4asv6);
 
 			if (pkt_result != PACKET_OK
diff --git a/src/itrafmon.c b/src/itrafmon.c
index c32c19d..6d555d1 100644
--- a/src/itrafmon.c
+++ b/src/itrafmon.c
@@ -835,7 +835,7 @@ void ipmon(struct OPTIONS *options, struct filterstate *ofilter,
 			rotate_flag = 0;
 		}
 
-		getpacket(fd, tpacket, &fromaddr, &ch, &readlen, NULL,
+		getpacket(fd, tpacket, &fromaddr, &ch, &readlen,
 			  table.tcpscreen);
 
 		if (ch == ERR)
@@ -1044,7 +1044,7 @@ void ipmon(struct OPTIONS *options, struct filterstate *ofilter,
 				  (unsigned int *) &readlen, &br,
 				  &sport, &dport, &fromaddr,
 				  ofilter, MATCH_OPPOSITE_ALWAYS,
-				  NULL, options->v6inv4asv6);
+				  options->v6inv4asv6);
 
 		if (pkt_result != PACKET_OK)
 			continue;
diff --git a/src/packet.c b/src/packet.c
index 0753ee0..f007b99 100644
--- a/src/packet.c
+++ b/src/packet.c
@@ -123,13 +123,12 @@ static void adjustpacket(char *tpacket, struct sockaddr_ll
*fromaddr,
  */
 
 void getpacket(int fd, char *buf, struct sockaddr_ll *fromaddr, int *ch,
-	       int *br, char *ifname, WINDOW * win)
+	       int *br, WINDOW * win)
 {
 	socklen_t fromlen;
 	fd_set set;
 	struct timeval tv;
 	int ss;
-	struct ifreq ifr;
 
 	FD_ZERO(&set);
 
@@ -160,11 +159,6 @@ void getpacket(int fd, char *buf, struct sockaddr_ll *fromaddr, int
*ch,
 		fromlen = sizeof(struct sockaddr_ll);
 		*br = recvfrom(fd, buf, MAX_PACKET_SIZE, 0,
 			       (struct sockaddr *) fromaddr, &fromlen);
-		if (ifname) {
-			ifr.ifr_ifindex = fromaddr->sll_ifindex;
-			ioctl(fd, SIOCGIFNAME, &ifr);
-			strcpy(ifname, ifr.ifr_name);
-		}
 	}
 	if (!daemonized && FD_ISSET(0, &set))
 		*ch = wgetch(win);
@@ -174,16 +168,8 @@ int processpacket(char *tpacket, char **packet, unsigned int *br,
 		  unsigned int *total_br, unsigned int *sport,
 		  unsigned int *dport, struct sockaddr_ll *fromaddr,
 		  struct filterstate *filter,
-		  int match_opposite, char *ifname, int v6inv4asv6)
+		  int match_opposite, int v6inv4asv6)
 {
-#if 0				/* reenable isdn */
-	/*
-	 * Prepare ISDN reference descriptor and table.
-	 */
-
-	memset(&isdntable, 0, sizeof(struct isdntab));
-	isdn_iface_check(&isdnfd, ifname);
-#endif
 	/*
 	 * Get IPTraf link type based on returned information and move past
 	 * data link header.
diff --git a/src/packet.h b/src/packet.h
index 22f9a67..d9a0915 100644
--- a/src/packet.h
+++ b/src/packet.h
@@ -27,12 +27,12 @@ extern int isdnfd;
 
 void open_socket(int *fd);
 void getpacket(int fd, char *buf, struct sockaddr_ll *fromaddr, int *ch,
-	       int *br, char *ifname, WINDOW * win);
+	       int *br, WINDOW * win);
 int processpacket(char *tpacket, char **packet, unsigned int *br,
 		  unsigned int *total_br, unsigned int *sport,
 		  unsigned int *dport, struct sockaddr_ll *fromaddr,
 		  struct filterstate *ofilter,
-		  int match_opposite, char *ifname, int v6inv4asv6);
+		  int match_opposite, int v6inv4asv6);
 void pkt_cleanup(void);
 
 #endif	/* IPTRAF_NG_PACKET_H */
diff --git a/src/pktsize.c b/src/pktsize.c
index ce0fd78..0612fbf 100644
--- a/src/pktsize.c
+++ b/src/pktsize.c
@@ -293,7 +293,7 @@ void packet_size_breakdown(struct OPTIONS *options, char *ifname,
 		    && (((now - starttime) / 60) >= facilitytime))
 			exitloop = 1;
 
-		getpacket(fd, buf, &fromaddr, &ch, &br, NULL, win);
+		getpacket(fd, buf, &fromaddr, &ch, &br, win);
 
 		if (ch != ERR) {
 			switch (ch) {
@@ -316,8 +316,7 @@ void packet_size_breakdown(struct OPTIONS *options, char *ifname,
 			    processpacket(buf, &ipacket, (unsigned int *) &br,
 					  NULL, NULL, NULL, &fromaddr,
 					  ofilter,
-					  MATCH_OPPOSITE_USECONFIG, NULL,
-					  0);
+					  MATCH_OPPOSITE_USECONFIG, 0);
 
 			if (pkt_result != PACKET_OK)
 				continue;
diff --git a/src/serv.c b/src/serv.c
index f0cd39f..c762104 100644
--- a/src/serv.c
+++ b/src/serv.c
@@ -862,7 +862,7 @@ void servmon(char *ifname, struct porttab *ports, const struct OPTIONS
*options,
 		    && (((now - starttime) / 60) >= facilitytime))
 			exitloop = 1;
 
-		getpacket(fd, buf, &fromaddr, &ch, &br, NULL, list.win);
+		getpacket(fd, buf, &fromaddr, &ch, &br, list.win);
 
 		if (ch == ERR)
 			goto no_key_ready;
@@ -997,7 +997,7 @@ void servmon(char *ifname, struct porttab *ports, const struct OPTIONS
*options,
 			processpacket(buf, &ipacket, (unsigned int *) &br,
 				      &tot_br, &sport, &dport, &fromaddr,
 				      ofilter,
-				      MATCH_OPPOSITE_USECONFIG, NULL,
+				      MATCH_OPPOSITE_USECONFIG,
 				      options->v6inv4asv6);
 
 		if (pkt_result != PACKET_OK)
-- 
1.7.8.4