Date: Wed, 11 Jun 2003 12:58:09 +0200 (CEST) From: Harti Brandt <brandt@fokus.fraunhofer.de> To: Mark Murray <mark@grondar.org> Cc: audit@freebsd.org Subject: Re: libatm cleanup. Message-ID: <20030611124832.I50294@beagle.fokus.fraunhofer.de> In-Reply-To: <200306101916.h5AJGDHh072147@grimreaper.grondar.org> References: <200306101916.h5AJGDHh072147@grimreaper.grondar.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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@fokus.fraunhofer.de, harti@freebsd.org
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20030611124832.I50294>