libatm cleanup.

Harti Brandt brandt at fokus.fraunhofer.de
Wed Jun 11 03:58:16 PDT 2003


On Tue, 10 Jun 2003, Mark Murray wrote:

MM>Any objections to me committing the attached patch? This is a big
MM>linting of the code, which leaves it WARNS=9 clean ;-). ANSIfication
MM>is also done.
MM>
MM>This does NOT affect build infrastructure. It just makes the
MM>build cleaner.
MM>
MM>(YeahYeahYeah. I know the highest WARNS is less than that).

Only three comments:

MM> Index: ioctl_subr.c
MM> ===================================================================
MM> RCS file: /home/ncvs/src/lib/libatm/ioctl_subr.c,v
MM> retrieving revision 1.8
MM> diff -u -d -r1.8 ioctl_subr.c
MM> --- ioctl_subr.c	30 Sep 2002 09:18:54 -0000	1.8
MM> +++ ioctl_subr.c	10 Jun 2003 19:08:14 -0000

...

MM> @@ -85,9 +81,7 @@
MM>   *
MM>   */
MM>  int
MM> -do_info_ioctl(req, buf_len)
MM> -	struct atminfreq	*req;
MM> -	int 			buf_len;
MM> +do_info_ioctl(struct atminfreq *req, int buf_len)
MM>  {
MM>  	int	rc, s;
MM>  	caddr_t	buf;
MM> @@ -104,18 +98,18 @@
MM>  	 * Get memory for returned information
MM>  	 */
MM>  mem_retry:
MM> -	buf = malloc(buf_len);
MM> +	buf = malloc((size_t)buf_len);
MM>  	if (buf == NULL) {
MM>  		errno = ENOMEM;
MM>  		return(-1);
MM>  	}
MM> -	bzero(buf, buf_len);
MM> +	bzero(buf, (size_t)buf_len);
MM>
MM>  	/*
MM>  	 * Set the buffer address and length in the request
MM>  	 */
MM>  	req->air_buf_addr = buf;
MM> -	req->air_buf_len = buf_len;
MM> +	req->air_buf_len = (int)buf_len;

Why would you need to cast an int to int?

...

MM> Index: ip_addr.c
MM> ===================================================================
MM> RCS file: /home/ncvs/src/lib/libatm/ip_addr.c,v
MM> retrieving revision 1.8
MM> diff -u -d -r1.8 ip_addr.c
MM> --- ip_addr.c	25 Mar 2003 04:29:26 -0000	1.8
MM> +++ ip_addr.c	10 Jun 2003 18:52:34 -0000
MM> @@ -41,7 +41,6 @@
MM>  #include <net/if.h>
MM>  #include <netinet/in.h>
MM>  #include <arpa/inet.h>
MM> -#include <netatm/port.h>
MM>  #include <netatm/atm.h>
MM>  #include <netatm/atm_if.h>
MM>  #include <netatm/atm_sap.h>
MM> @@ -69,11 +68,11 @@
MM>   *
MM>   */
MM>  struct sockaddr_in *
MM> -get_ip_addr(p)
MM> -	char	*p;
MM> +get_ip_addr(const char *p)
MM>  {
MM>  	struct hostent			*ip_host;
MM>  	static struct sockaddr_in	s;
MM> +	u_long				*temp;
MM>
MM>  	/*
MM>  	 * Get IP address of specified host name
MM> @@ -96,7 +95,8 @@
MM>  				ip_host->h_addrtype != AF_INET) {
MM>  			return((struct sockaddr_in *)0);
MM>  		}
MM> -		s.sin_addr.s_addr = *(u_long *)ip_host->h_addr_list[0];
MM> +		temp = (u_long *)(void *)ip_host->h_addr_list[0];
MM> +		s.sin_addr.s_addr = (u_int)*temp;

I assume it would be better to do

		bcopy(ip_host->h_addr_list[0], &s.sin_addr.s_addr,
		    sizeof(s.sin_addr.s_addr));

here because of alignment issues (gethostbyname(3) doesn't say anything about
alignment so I think it is not wise to assume that a char ** can safely
be converted to a u_long * and be used to access that u_long).

MM>  	}
MM>  	return(&s);
MM>  }
MM> @@ -116,8 +116,7 @@
MM>   *
MM>   */
MM>  const char *
MM> -format_ip_addr(addr)
MM> -	struct in_addr	*addr;
MM> +format_ip_addr(const struct in_addr *addr)
MM>  {
MM>  	static char	host_name[128];
MM>  	char		*ip_num;
MM> @@ -132,7 +131,8 @@
MM>  	 * Check for a zero address
MM>  	 */
MM>  	if (!addr || addr->s_addr == 0) {
MM> -		return("-");
MM> +		strcpy(host_name, "-");
MM> +		return(host_name);
MM>  	}

What is the reason for this? "-" should be a perfect legal const char *.

harti
-- 
harti brandt,
http://www.fokus.fraunhofer.de/research/cc/cats/employees/hartmut.brandt/private
brandt at fokus.fraunhofer.de, harti at freebsd.org


More information about the freebsd-audit mailing list