From owner-freebsd-audit@FreeBSD.ORG Tue Jun 10 12:19:25 2003 Return-Path: Delivered-To: freebsd-audit@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 30A4C37B404 for ; Tue, 10 Jun 2003 12:19:23 -0700 (PDT) Received: from storm.FreeBSD.org.uk (storm.FreeBSD.org.uk [194.242.157.42]) by mx1.FreeBSD.org (Postfix) with ESMTP id 8EED843FDD for ; Tue, 10 Jun 2003 12:19:21 -0700 (PDT) (envelope-from mark@grondar.org) Received: from storm.FreeBSD.org.uk (Ugrondar@localhost [127.0.0.1]) by storm.FreeBSD.org.uk (8.12.9/8.12.9) with ESMTP id h5AJJKuD026062 for ; Tue, 10 Jun 2003 20:19:20 +0100 (BST) (envelope-from mark@grondar.org) Received: (from Ugrondar@localhost)h5AJJJn0026061 for audit@freebsd.org; Tue, 10 Jun 2003 20:19:19 +0100 (BST) X-Authentication-Warning: storm.FreeBSD.org.uk: Ugrondar set sender to mark@grondar.org using -f Received: from grondar.org (localhost [127.0.0.1]) by grimreaper.grondar.org (8.12.9/8.12.9) with ESMTP id h5AJGDHh072147 for ; Tue, 10 Jun 2003 20:16:13 +0100 (BST) (envelope-from mark@grondar.org) From: Mark Murray Message-Id: <200306101916.h5AJGDHh072147@grimreaper.grondar.org> To: audit@freebsd.org MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0" Content-ID: <72128.1055272499.0@grondar.org> Date: Tue, 10 Jun 2003 20:16:13 +0100 Sender: mark@grondar.org X-Spam-Status: No, hits=1.7 required=5.0 tests=FROM_NO_LOWER,PATCH_UNIFIED_DIFF version=2.55 X-Spam-Level: * X-Spam-Checker-Version: SpamAssassin 2.55 (1.174.2.19-2003-05-19-exp) Subject: libatm cleanup. X-BeenThere: freebsd-audit@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: FreeBSD Security Audit List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 10 Jun 2003 19:19:25 -0000 ------- =_aaaaaaaaaa0 Content-Type: text/plain; charset="us-ascii" Content-ID: <72128.1055272499.1@grondar.org> Hi all Any objections to me committing the attached patch? This is a big linting of the code, which leaves it WARNS=9 clean ;-). ANSIfication is also done. This does NOT affect build infrastructure. It just makes the build cleaner. (YeahYeahYeah. I know the highest WARNS is less than that). M -- Mark Murray iumop ap!sdn w,I idlaH ------- =_aaaaaaaaaa0 Content-Type: text/plain; charset="us-ascii" Content-ID: <72128.1055272499.2@grondar.org> Content-Description: libatm.diff Index: atm_addr.c =================================================================== RCS file: /home/ncvs/src/lib/libatm/atm_addr.c,v retrieving revision 1.10 diff -u -d -r1.10 atm_addr.c --- atm_addr.c 20 Apr 2003 18:41:16 -0000 1.10 +++ atm_addr.c 10 Jun 2003 18:42:29 -0000 @@ -29,6 +29,7 @@ static char *RCSid = "@(#) $Id: atm_addr.c,v 1.1 1998/07/09 21:45:18 johnc Exp $"; #endif #endif + #include __FBSDID("$FreeBSD: src/lib/libatm/atm_addr.c,v 1.10 2003/04/20 18:41:16 obrien Exp $"); @@ -45,7 +46,6 @@ #include #include #include -#include #include #include #include @@ -57,8 +57,6 @@ #include "libatm.h" -extern char *prog; - /* * Get NSAP, NSAP prefix or MAC address * @@ -72,11 +70,9 @@ * len the length of the data in the output buffer * */ +/* ARGSUSED */ int -get_hex_atm_addr(in, out, len) - char *in; - u_char *out; - int len; +get_hex_atm_addr(char *in, u_char *out, int len __unused) { int c_type, c_value, i, out_len, state, val = 0; @@ -140,7 +136,7 @@ /* * Loop through input until state table says to return */ - while (1) { + for (;;) { /* * Get the character type and value */ @@ -219,8 +215,7 @@ * */ char * -format_atm_addr(addr) - Atm_addr *addr; +format_atm_addr(Atm_addr *addr) { int i; char *nsap_format; @@ -231,7 +226,7 @@ static char str[256]; union { int w; - char c[4]; + u_char c[4]; } u1, u2; static char nsap_format_DCC[] = "0x%02x.%02x%02x.%02x.%02x%02x%02x.%02x%02x.%02x%02x.%02x%02x.%02x%02x%02x%02x%02x%02x.%02x"; @@ -310,7 +305,8 @@ if (!(u1.w == 0 && u2.w == 0)) sprintf(str, "0x%08lx.%08lx", - (u_long)ntohl(u1.w), (u_long)ntohl(u2.w)); + (u_long)ntohl((u_int)u1.w), + (u_long)ntohl((u_int)u2.w)); break; case T_ATM_PVC_ADDR: Index: cache_key.c =================================================================== RCS file: /home/ncvs/src/lib/libatm/cache_key.c,v retrieving revision 1.8 diff -u -d -r1.8 cache_key.c --- cache_key.c 25 Mar 2003 04:29:26 -0000 1.8 +++ cache_key.c 10 Jun 2003 18:43:25 -0000 @@ -40,7 +40,6 @@ #include #include #include -#include #include #include #include @@ -66,14 +65,11 @@ * */ void -scsp_cache_key(ap, ip, ol, op) - Atm_addr *ap; - struct in_addr *ip; - int ol; - char *op; +scsp_cache_key(Atm_addr *ap, struct in_addr *ip, int ol, char *op) { - int i, len; - char buff[32], digest[16]; + int i; + size_t len; + u_char buff[32], digest[16]; MD5_CTX context; /* @@ -101,7 +97,7 @@ /* * Fold the 16-byte digest to the required length */ - bzero((caddr_t)op, ol); + bzero((caddr_t)op, (size_t)ol); for (i = 0; i < 16; i++) { op[i % ol] = op[i % ol] ^ digest[i]; } Index: ioctl_subr.c =================================================================== RCS file: /home/ncvs/src/lib/libatm/ioctl_subr.c,v retrieving revision 1.8 diff -u -d -r1.8 ioctl_subr.c --- ioctl_subr.c 30 Sep 2002 09:18:54 -0000 1.8 +++ ioctl_subr.c 10 Jun 2003 19:08:14 -0000 @@ -41,7 +41,6 @@ #include #include #include -#include #include #include #include @@ -63,9 +62,6 @@ #define FALSE 0 #endif -extern char *prog; - - /* * Issue an informational IOCTL * @@ -85,9 +81,7 @@ * */ int -do_info_ioctl(req, buf_len) - struct atminfreq *req; - int buf_len; +do_info_ioctl(struct atminfreq *req, int buf_len) { int rc, s; caddr_t buf; @@ -104,18 +98,18 @@ * Get memory for returned information */ mem_retry: - buf = malloc(buf_len); + buf = malloc((size_t)buf_len); if (buf == NULL) { errno = ENOMEM; return(-1); } - bzero(buf, buf_len); + bzero(buf, (size_t)buf_len); /* * Set the buffer address and length in the request */ req->air_buf_addr = buf; - req->air_buf_len = buf_len; + req->air_buf_len = (int)buf_len; /* * Issue the IOCTL @@ -152,11 +146,9 @@ * */ int -get_vcc_info(intf, vccp) - char *intf; - struct air_vcc_rsp **vccp; +get_vcc_info(const char *intf, struct air_vcc_rsp **vccp) { - int buf_len = sizeof(struct air_vcc_rsp) * 100; + int buf_len = (int)sizeof(struct air_vcc_rsp) * 100; struct atminfreq air; /* @@ -190,9 +182,7 @@ * */ int -get_subnet_mask(intf, mask) - char *intf; - struct sockaddr_in *mask; +get_subnet_mask(const char *intf, struct sockaddr_in *mask) { int rc, s; struct ifreq req; @@ -247,8 +237,7 @@ * */ int -get_mtu(intf) - char *intf; +get_mtu(const char *intf) { int rc, s; struct ifreq req; @@ -281,7 +270,7 @@ if (rc) return(-1); else - return(req.ifr_mtu); + return(req.ifr_mtu); } @@ -301,8 +290,7 @@ * */ int -verify_nif_name(name) - char *name; +verify_nif_name(const char *name) { int rc, s; struct atminfreq air; @@ -372,11 +360,9 @@ * */ int -get_cfg_info ( intf, cfgp ) - char *intf; - struct air_cfg_rsp **cfgp; +get_cfg_info (const char *intf, struct air_cfg_rsp **cfgp) { - int buf_len = sizeof(struct air_cfg_rsp) * 4; + int buf_len = (int)sizeof(struct air_cfg_rsp) * 4; struct atminfreq air; /* @@ -410,11 +396,9 @@ * */ int -get_intf_info ( intf, intp ) - char *intf; - struct air_int_rsp **intp; +get_intf_info (const char *intf, struct air_int_rsp **intp) { - int buf_len = sizeof(struct air_int_rsp) * 4; + int buf_len = (int)sizeof(struct air_int_rsp) * 4; struct atminfreq air; /* @@ -449,11 +433,9 @@ * */ int -get_netif_info ( intf, netp ) - char *intf; - struct air_netif_rsp **netp; +get_netif_info (const char *intf, struct air_netif_rsp **netp) { - int buf_len = sizeof(struct air_netif_rsp) * 10; + int buf_len = (int)sizeof(struct air_netif_rsp) * 10; struct atminfreq air; /* Index: ip_addr.c =================================================================== RCS file: /home/ncvs/src/lib/libatm/ip_addr.c,v retrieving revision 1.8 diff -u -d -r1.8 ip_addr.c --- ip_addr.c 25 Mar 2003 04:29:26 -0000 1.8 +++ ip_addr.c 10 Jun 2003 18:52:34 -0000 @@ -41,7 +41,6 @@ #include #include #include -#include #include #include #include @@ -69,11 +68,11 @@ * */ struct sockaddr_in * -get_ip_addr(p) - char *p; +get_ip_addr(const char *p) { struct hostent *ip_host; static struct sockaddr_in s; + u_long *temp; /* * Get IP address of specified host name @@ -96,7 +95,8 @@ ip_host->h_addrtype != AF_INET) { return((struct sockaddr_in *)0); } - s.sin_addr.s_addr = *(u_long *)ip_host->h_addr_list[0]; + temp = (u_long *)(void *)ip_host->h_addr_list[0]; + s.sin_addr.s_addr = (u_int)*temp; } return(&s); } @@ -116,8 +116,7 @@ * */ const char * -format_ip_addr(addr) - struct in_addr *addr; +format_ip_addr(const struct in_addr *addr) { static char host_name[128]; char *ip_num; @@ -132,7 +131,8 @@ * Check for a zero address */ if (!addr || addr->s_addr == 0) { - return("-"); + strcpy(host_name, "-"); + return(host_name); } /* @@ -143,7 +143,7 @@ /* * Look up name in DNS */ - ip_host = gethostbyaddr((char *)addr, sizeof(addr), AF_INET); + ip_host = gethostbyaddr((const char *)addr, sizeof(addr), AF_INET); if (ip_host && ip_host->h_name && strlen(ip_host->h_name)) { /* Index: ip_checksum.c =================================================================== RCS file: /home/ncvs/src/lib/libatm/ip_checksum.c,v retrieving revision 1.7 diff -u -d -r1.7 ip_checksum.c --- ip_checksum.c 21 Mar 2002 23:35:20 -0000 1.7 +++ ip_checksum.c 10 Jun 2003 18:48:34 -0000 @@ -40,7 +40,6 @@ #include #include #include -#include #include #include #include @@ -66,25 +65,24 @@ * */ short -ip_checksum(addr, count) - char *addr; - int count; +ip_checksum(char *addr, int count) { /* Compute Internet Checksum for "count" bytes * beginning at location "addr". */ - long sum = 0; + unsigned long sum = 0; + unsigned short *laddr; + laddr = (unsigned short *)(void *)addr; while( count > 1 ) { /* This is the inner loop */ - sum += ntohs(* (unsigned short *) addr); - addr += sizeof(unsigned short); - count -= sizeof(unsigned short); + sum += ntohs(*(laddr++)); + count -= (int)sizeof(unsigned short); } /* Add left-over byte, if any */ if( count > 0 ) - sum += * (unsigned char *) addr; + sum += *laddr; /* Fold 32-bit sum to 16 bits */ while (sum>>16) Index: libatm.h =================================================================== RCS file: /home/ncvs/src/lib/libatm/libatm.h,v retrieving revision 1.6 diff -u -d -r1.6 libatm.h --- libatm.h 25 Mar 2003 04:29:26 -0000 1.6 +++ libatm.h 10 Jun 2003 19:07:29 -0000 @@ -90,17 +90,17 @@ /* ioctl_subr.c */ extern int do_info_ioctl(struct atminfreq *, int); -extern int get_vcc_info(char *, struct air_vcc_rsp **); -extern int get_subnet_mask(char *, struct sockaddr_in *); -extern int get_mtu(char *); -extern int verify_nif_name(char *); -extern int get_cfg_info(char *, struct air_cfg_rsp **); -extern int get_intf_info(char *, struct air_int_rsp **); -extern int get_netif_info(char *, struct air_netif_rsp **); +extern int get_vcc_info(const char *, struct air_vcc_rsp **); +extern int get_subnet_mask(const char *, struct sockaddr_in *); +extern int get_mtu(const char *); +extern int verify_nif_name(const char *); +extern int get_cfg_info(const char *, struct air_cfg_rsp **); +extern int get_intf_info(const char *, struct air_int_rsp **); +extern int get_netif_info(const char *, struct air_netif_rsp **); /* ip_addr.c */ -extern struct sockaddr_in *get_ip_addr(char *); -extern const char *format_ip_addr(struct in_addr *); +extern struct sockaddr_in *get_ip_addr(const char *); +extern const char *format_ip_addr(const struct in_addr *); /* ip_checksum.c */ extern short ip_checksum(char *, int); Index: timer.c =================================================================== RCS file: /home/ncvs/src/lib/libatm/timer.c,v retrieving revision 1.8 diff -u -d -r1.8 timer.c --- timer.c 25 Mar 2003 04:29:26 -0000 1.8 +++ timer.c 10 Jun 2003 18:49:18 -0000 @@ -40,7 +40,6 @@ #include #include #include -#include #include #include #include @@ -175,7 +174,7 @@ * */ int -init_timer() +init_timer(void) { int rc = 0; struct itimerval timeval; @@ -223,7 +222,7 @@ * */ int -block_timer() +block_timer(void) { /* * Block the SIGALRM signal @@ -246,8 +245,7 @@ * */ void -enable_timer(mask) - int mask; +enable_timer(int mask) { /* * Set the signal mask ------- =_aaaaaaaaaa0-- From owner-freebsd-audit@FreeBSD.ORG Wed Jun 11 03:58:16 2003 Return-Path: Delivered-To: freebsd-audit@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 7144937B401 for ; Wed, 11 Jun 2003 03:58:16 -0700 (PDT) Received: from mailhub.fokus.fraunhofer.de (mailhub.fokus.fraunhofer.de [193.174.154.14]) by mx1.FreeBSD.org (Postfix) with ESMTP id 5991643FCB for ; Wed, 11 Jun 2003 03:58:15 -0700 (PDT) (envelope-from brandt@fokus.fraunhofer.de) Received: from beagle (beagle [193.175.132.100])h5BAw9d09416; Wed, 11 Jun 2003 12:58:12 +0200 (MEST) Date: Wed, 11 Jun 2003 12:58:09 +0200 (CEST) From: Harti Brandt To: Mark Murray In-Reply-To: <200306101916.h5AJGDHh072147@grimreaper.grondar.org> Message-ID: <20030611124832.I50294@beagle.fokus.fraunhofer.de> References: <200306101916.h5AJGDHh072147@grimreaper.grondar.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII cc: audit@freebsd.org Subject: Re: libatm cleanup. X-BeenThere: freebsd-audit@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: FreeBSD Security Audit List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 11 Jun 2003 10:58:16 -0000 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 MM> #include MM> #include MM> -#include MM> #include MM> #include MM> #include 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@fokus.fraunhofer.de, harti@freebsd.org