Hello, everyone, I have a preliminary patch against rpc.rstatd to get it WARNS=6 clean on both i386 and all Tier-1 64-bit platforms. Since I'm not quite familiar with the rpc* code I would like to solicit a review on the patch, see attachment.
Changes include:
- Avoid 64-bit platform complains due to timeval width
changes, by introducing an temporary newcurtime when
obtaining the current time, since rstat_timeval uses
u_int while timeval uses long. (value could be truncated
but that's better than overwriting unwanted area)
- Use xdrproc_t instead of the more complicated definations
(and which doesn't provided enough information for compiler
to generate correct code if optimization level is turned
too high)
- Initialize variables that can potentially be used without
initialization.
- Explicitly cast when necessary
- Use NULL in favor of 0 when representing a NULL pointer
- Don't use constant strings for initialization of char *
- Add a missing prototype for rstat_service
- Prefer complete prototype over incomplete ones.
- Use static where applicable.
- Apply __unused on unused parameters
- Avoid shadowed names
- Bump WARNS?= from default to 6
Thanks in advance!
Cheers,
--
Xin LI <delphij delphij net> http://www.delphij.net/
Index: Makefile
===================================================================
RCS file: /home/ncvs/src/libexec/rpc.rstatd/Makefile,v
retrieving revision 1.8
diff -u -r1.8 Makefile
--- Makefile 4 Feb 2004 10:20:43 -0000 1.8
+++ Makefile 20 Jan 2005 14:44:46 -0000
@@ -4,6 +4,8 @@
SRCS = rstatd.c rstat_proc.c
MAN = rpc.rstatd.8
+WARNS?= 6
+
DPADD= ${LIBRPCSVC} ${LIBUTIL} ${LIBDEVSTAT} ${LIBKVM}
LDADD= -lrpcsvc -lutil -ldevstat -lkvm
Index: rstat_proc.c
===================================================================
RCS file: /home/ncvs/src/libexec/rpc.rstatd/rstat_proc.c,v
retrieving revision 1.25
diff -u -r1.25 rstat_proc.c
--- rstat_proc.c 2 Jun 2003 02:34:36 -0000 1.25
+++ rstat_proc.c 20 Jan 2005 15:38:56 -0000
@@ -76,18 +76,22 @@
#undef if_collisions
#include <rpcsvc/rstat.h>
+static char strcptime[] = "_cp_time";
+static char strcnt[] = "_cnt";
+
struct nlist nl[] = {
#define X_CPTIME 0
- { "_cp_time" },
+ { strcptime, 0, 0, 0, 0 },
#define X_CNT 1
- { "_cnt" },
- { "" },
+ { strcnt, 0, 0, 0, 0 },
+ { NULL, 0, 0, 0, 0 },
};
int haveadisk(void);
void updatexfers(int, int *);
void setup(void);
int stats_service(void);
+void rstat_service(struct svc_req *, SVCXPRT *);
extern int from_inetd;
int sincelastreq = 0; /* number of alarms since last request */
@@ -99,7 +103,7 @@
struct statstime s3;
} stats_all;
-void updatestat();
+void updatestat(int);
static int stat_is_init = 0;
static kvm_t *kd;
@@ -112,19 +116,19 @@
#define FSCALE (1 << 8)
#endif
-void
+static void
stat_init(void)
{
stat_is_init = 1;
setup();
alarm(0);
- updatestat();
- (void) signal(SIGALRM, updatestat);
+ updatestat(0);
+ signal(SIGALRM, updatestat);
alarm(1);
}
statstime *
-rstatproc_stats_3_svc(void *argp, struct svc_req *rqstp)
+rstatproc_stats_3_svc(__unused void *argp, __unused struct svc_req *rqstp)
{
if (! stat_is_init)
stat_init();
@@ -133,7 +137,7 @@
}
statsswtch *
-rstatproc_stats_2_svc(void *argp, struct svc_req *rqstp)
+rstatproc_stats_2_svc(__unused void *argp, __unused struct svc_req *rqstp)
{
if (! stat_is_init)
stat_init();
@@ -142,7 +146,7 @@
}
stats *
-rstatproc_stats_1_svc(void *argp, struct svc_req *rqstp)
+rstatproc_stats_1_svc(__unused void *argp, __unused struct svc_req *rqstp)
{
if (! stat_is_init)
stat_init();
@@ -151,7 +155,7 @@
}
u_int *
-rstatproc_havedisk_3_svc(void *argp, struct svc_req *rqstp)
+rstatproc_havedisk_3_svc(__unused void *argp, __unused struct svc_req *rqstp)
{
static u_int have;
@@ -175,12 +179,13 @@
}
void
-updatestat(void)
+updatestat(__unused int dummy)
{
int i, hz;
struct clockinfo clockrate;
struct vmmeter cnt;
struct ifmibdata ifmd;
+ struct timeval newcurtime; /* XXX 64-bit */
double avrun[3];
struct timeval tm, btm;
int mib[6];
@@ -221,7 +226,7 @@
for(i = 0; i < RSTAT_CPUSTATES ; i++)
stats_all.s1.cp_time[i] = bsd_cp_time[cp_time_xlat[i]];
- (void)getloadavg(avrun, sizeof(avrun) / sizeof(avrun[0]));
+ getloadavg(avrun, sizeof(avrun) / sizeof(avrun[0]));
stats_all.s2.avenrun[0] = avrun[0] * FSCALE;
stats_all.s2.avenrun[1] = avrun[1] * FSCALE;
@@ -254,7 +259,7 @@
stats_all.s1.v_pswpin = cnt.v_swappgsin;
stats_all.s1.v_pswpout = cnt.v_swappgsout;
stats_all.s1.v_intr = cnt.v_intr;
- gettimeofday(&tm, (struct timezone *) 0);
+ gettimeofday(&tm, (struct timezone *) NULL);
stats_all.s1.v_intr -= hz*(tm.tv_sec - btm.tv_sec) +
hz*(tm.tv_usec - btm.tv_usec)/1000000;
stats_all.s2.v_swtch = cnt.v_swtch;
@@ -298,8 +303,9 @@
stats_all.s1.if_oerrors += ifmd.ifmd_data.ifi_oerrors;
stats_all.s1.if_collisions += ifmd.ifmd_data.ifi_collisions;
}
- gettimeofday((struct timeval *)&stats_all.s3.curtime,
- (struct timezone *) 0);
+ gettimeofday(&newcurtime, (struct timezone *) NULL);
+ stats_all.s3.curtime.tv_sec = newcurtime.tv_sec;
+ stats_all.s3.curtime.tv_usec = newcurtime.tv_usec;
alarm(1);
}
@@ -328,7 +334,7 @@
haveadisk(void)
{
register int i;
- struct statinfo stats;
+ struct statinfo diskstats;
int num_devices, retval = 0;
if ((num_devices = devstat_getnumdevs(NULL)) < 0) {
@@ -342,28 +348,28 @@
exit(1);
}
- stats.dinfo = (struct devinfo *)malloc(sizeof(struct devinfo));
- bzero(stats.dinfo, sizeof(struct devinfo));
+ diskstats.dinfo = malloc(sizeof(*diskstats.dinfo));
+ bzero(diskstats.dinfo, sizeof(*diskstats.dinfo));
- if (devstat_getdevs(NULL, &stats) == -1) {
+ if (devstat_getdevs(NULL, &diskstats) == -1) {
syslog(LOG_ERR, "rstatd: can't get device list: %s",
devstat_errbuf);
exit(1);
}
- for (i = 0; i < stats.dinfo->numdevs; i++) {
- if (((stats.dinfo->devices[i].device_type
+ for (i = 0; i < diskstats.dinfo->numdevs; i++) {
+ if (((diskstats.dinfo->devices[i].device_type
& DEVSTAT_TYPE_MASK) == DEVSTAT_TYPE_DIRECT)
- && ((stats.dinfo->devices[i].device_type
+ && ((diskstats.dinfo->devices[i].device_type
& DEVSTAT_TYPE_PASS) == 0)) {
retval = 1;
break;
}
}
- if (stats.dinfo->mem_ptr)
- free(stats.dinfo->mem_ptr);
+ if (diskstats.dinfo->mem_ptr)
+ free(diskstats.dinfo->mem_ptr);
- free(stats.dinfo);
+ free(diskstats.dinfo);
return(retval);
}
@@ -371,7 +377,7 @@
updatexfers(int numdevs, int *devs)
{
register int i, j, k, t;
- struct statinfo stats;
+ struct statinfo diskstats;
int num_devices = 0;
u_int64_t total_transfers;
@@ -386,24 +392,24 @@
exit(1);
}
- stats.dinfo = (struct devinfo *)malloc(sizeof(struct devinfo));
- bzero(stats.dinfo, sizeof(struct devinfo));
+ diskstats.dinfo = malloc(sizeof(*diskstats.dinfo));
+ bzero(diskstats.dinfo, sizeof(*diskstats.dinfo));
- if (devstat_getdevs(NULL, &stats) == -1) {
+ if (devstat_getdevs(NULL, &diskstats) == -1) {
syslog(LOG_ERR, "rstatd: can't get device list: %s",
devstat_errbuf);
exit(1);
}
- for (i = 0, j = 0; i < stats.dinfo->numdevs && j < numdevs; i++) {
- if (((stats.dinfo->devices[i].device_type
+ for (i = 0, j = 0; i < diskstats.dinfo->numdevs && j < numdevs; i++) {
+ if (((diskstats.dinfo->devices[i].device_type
& DEVSTAT_TYPE_MASK) == DEVSTAT_TYPE_DIRECT)
- && ((stats.dinfo->devices[i].device_type
+ && ((diskstats.dinfo->devices[i].device_type
& DEVSTAT_TYPE_PASS) == 0)) {
total_transfers = 0;
for (k = 0; k < DEVSTAT_N_TRANS_FLAGS; k++)
total_transfers +=
- stats.dinfo->devices[i].operations[k];
+ diskstats.dinfo->devices[i].operations[k];
/*
* XXX KDM If the total transfers for this device
* are greater than the amount we can fit in a
@@ -422,10 +428,10 @@
}
}
- if (stats.dinfo->mem_ptr)
- free(stats.dinfo->mem_ptr);
+ if (diskstats.dinfo->mem_ptr)
+ free(diskstats.dinfo->mem_ptr);
- free(stats.dinfo);
+ free(diskstats.dinfo);
}
void
@@ -435,26 +441,26 @@
int fill;
} argument;
char *result;
- bool_t (*xdr_argument)(), (*xdr_result)();
- char *(*local)();
-
+ xdrproc_t xdr_argument, xdr_result;
+ typedef void *(*rsvcfp_t)(void *, struct svc_req *);
+ rsvcfp_t local;
switch (rqstp->rq_proc) {
case NULLPROC:
- (void)svc_sendreply(transp, (xdrproc_t)xdr_void, NULL);
+ svc_sendreply(transp, (xdrproc_t)xdr_void, NULL);
goto leave;
case RSTATPROC_STATS:
- xdr_argument = xdr_void;
- xdr_result = xdr_statstime;
+ xdr_argument = (xdrproc_t)xdr_void;
+ xdr_result = (xdrproc_t)xdr_statstime;
switch (rqstp->rq_vers) {
case RSTATVERS_ORIG:
- local = (char *(*)()) rstatproc_stats_1_svc;
+ local = (rsvcfp_t) rstatproc_stats_1_svc;
break;
case RSTATVERS_SWTCH:
- local = (char *(*)()) rstatproc_stats_2_svc;
+ local = (rsvcfp_t) rstatproc_stats_2_svc;
break;
case RSTATVERS_TIME:
- local = (char *(*)()) rstatproc_stats_3_svc;
+ local = (rsvcfp_t) rstatproc_stats_3_svc;
break;
default:
svcerr_progvers(transp, RSTATVERS_ORIG, RSTATVERS_TIME);
@@ -464,17 +470,17 @@
break;
case RSTATPROC_HAVEDISK:
- xdr_argument = xdr_void;
- xdr_result = xdr_u_int;
+ xdr_argument = (xdrproc_t)xdr_void;
+ xdr_result = (xdrproc_t)xdr_u_int;
switch (rqstp->rq_vers) {
case RSTATVERS_ORIG:
- local = (char *(*)()) rstatproc_havedisk_1_svc;
+ local = (rsvcfp_t) rstatproc_havedisk_1_svc;
break;
case RSTATVERS_SWTCH:
- local = (char *(*)()) rstatproc_havedisk_2_svc;
+ local = (rsvcfp_t) rstatproc_havedisk_2_svc;
break;
case RSTATVERS_TIME:
- local = (char *(*)()) rstatproc_havedisk_3_svc;
+ local = (rsvcfp_t) rstatproc_havedisk_3_svc;
break;
default:
svcerr_progvers(transp, RSTATVERS_ORIG, RSTATVERS_TIME);
@@ -487,7 +493,7 @@
svcerr_noproc(transp);
goto leave;
}
- bzero((char *)&argument, sizeof(argument));
+ bzero(&argument, sizeof(argument));
if (!svc_getargs(transp, (xdrproc_t)xdr_argument, &argument)) {
svcerr_decode(transp);
goto leave;
Index: rstatd.c
===================================================================
RCS file: /home/ncvs/src/libexec/rpc.rstatd/rstatd.c,v
retrieving revision 1.10
diff -u -r1.10 rstatd.c
--- rstatd.c 15 Jul 2002 18:51:57 -0000 1.10
+++ rstatd.c 20 Jan 2005 15:44:25 -0000
@@ -47,22 +47,22 @@
int from_inetd = 1; /* started from inetd ? */
int closedown = 20; /* how long to wait before going dormant */
-void
+static void
cleanup(int sig __unused)
{
- (void) rpcb_unset(RSTATPROG, RSTATVERS_TIME, NULL);
- (void) rpcb_unset(RSTATPROG, RSTATVERS_SWTCH, NULL);
- (void) rpcb_unset(RSTATPROG, RSTATVERS_ORIG, NULL);
+ rpcb_unset(RSTATPROG, RSTATVERS_TIME, NULL);
+ rpcb_unset(RSTATPROG, RSTATVERS_SWTCH, NULL);
+ rpcb_unset(RSTATPROG, RSTATVERS_ORIG, NULL);
exit(0);
}
int
main(int argc, char *argv[])
{
- SVCXPRT *transp;
+ SVCXPRT *transp = NULL;
int ok;
struct sockaddr_storage from;
- int fromlen;
+ socklen_t fromlen;
if (argc == 2)
closedown = atoi(argv[1]);
@@ -78,15 +78,15 @@
}
if (!from_inetd) {
- daemon(0, 0);
+ daemon(0, 0);
- (void)rpcb_unset(RSTATPROG, RSTATVERS_TIME, NULL);
- (void)rpcb_unset(RSTATPROG, RSTATVERS_SWTCH, NULL);
- (void)rpcb_unset(RSTATPROG, RSTATVERS_ORIG, NULL);
-
- (void) signal(SIGINT, cleanup);
- (void) signal(SIGTERM, cleanup);
- (void) signal(SIGHUP, cleanup);
+ rpcb_unset(RSTATPROG, RSTATVERS_TIME, NULL);
+ rpcb_unset(RSTATPROG, RSTATVERS_SWTCH, NULL);
+ rpcb_unset(RSTATPROG, RSTATVERS_ORIG, NULL);
+
+ signal(SIGINT, cleanup);
+ signal(SIGTERM, cleanup);
+ signal(SIGHUP, cleanup);
}
openlog("rpc.rstatd", LOG_CONS|LOG_PID, LOG_DAEMON);
signature.asc
Description: =?UTF-8?Q?=E8=BF=99=E6=98=AF=E4=BF=A1=E4=BB=B6=E7=9A=84=E6=95=B0?= =?UTF-8?Q?=E5=AD=97=E7=AD=BE=E5=90=8D=E9=83=A8?= =?UTF-8?Q?=E5=88=86?=
