Turn on the -Wextra compile flag
Subsystems (2)
cover Turn on the -Wextra compile flag
To catch potential during compile time, the -Wextra compile flag was added to the CFLAGS. The raise the bar like this, with regard to warnings, will keep the code cleaner in the long run.
Steve Dickson (24):
Enable extra compile warnings (-Wextra) by default. Remove warnings from nfsctl.c
Removed warnings from rpcdispatch.c
Remove warnings from svc_socket.c
emove warnings from cacheio.c
Remove warnings from nfs_mntent.c
Removed warnings from conffile.c
Removed warnings from mountd.c
Removed warnings from fsloc.c
Removed warnings from cache.c
Removed warnings from nfssvc.c
Removed warnings from nfsstat.c
Removed warnings from atomicio.c
Removed warnings from idmapd.c
Removed warnings from gssd.c
Removed warnings from gssd_main_loop.c Removed warnings from krb5_util.c
Removed warnings from gssd_proc.c
Removed warnings from svcgssd.c
Removed warnings from svcgssd_proc.c Removed warnings from network.c
Removed warnings from nfsmount.c
Removed warnings from nfs4mount.c
Removed warnings from configfile.c
configure.ac | 2 +- support/export/nfsctl.c | 2 +- support/include/nfslib.h | 4 ++-- support/nfs/cacheio.c | 2 +- support/nfs/conffile.c | 8 ++++---- support/nfs/nfs_mntent.c | 2 +- support/nfs/rpcdispatch.c | 4 ++-- support/nfs/rpcmisc.c | 2 +- support/nfs/svc_socket.c | 6 +++--- utils/gssd/gssd.c | 2 +- utils/gssd/gssd_main_loop.c | 2 ++ utils/gssd/gssd_proc.c | 8 ++++---- utils/gssd/krb5_util.c | 29 +++++++++++++++++++++-------- utils/gssd/krb5_util.h | 2 +- utils/gssd/svcgssd.c | 2 +- utils/gssd/svcgssd_proc.c | 8 ++++---- utils/idmapd/atomicio.c | 3 ++- utils/idmapd/idmapd.c | 36 +++++++++++++++++++++++++++++++----- utils/mount/configfile.c | 4 ++-- utils/mount/network.c | 19 +++++++++---------- utils/mount/nfs4mount.c | 2 +- utils/mount/nfsmount.c | 8 ++++++-- utils/mountd/cache.c | 20 ++++++++++---------- utils/mountd/fsloc.c | 2 +- utils/mountd/fsloc.h | 2 +- utils/mountd/mountd.c | 19 +++++++++++++++---- utils/nfsd/nfssvc.c | 6 +++--- utils/nfsstat/nfsstat.c | 6 +++--- 28 files changed, 134 insertions(+), 78 deletions(-)
PATCH 1/24 Enable extra compile warnings (-Wextra) by default.
Added -Wextra to the CFLAGS which enables more checking during compilation, which in turn, will hopefully flag potential problems before they occur.
--- configure.ac | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
configure.ac +1 -1
@@ -400,7 +400,7 @@ case $host in ARCHFLAGS="" ;; esac -my_am_cflags="-Wall -Wstrict-prototypes $ARCHFLAGS -pipe" +my_am_cflags="-Wall -Wextra -Wstrict-prototypes $ARCHFLAGS -pipe" AC_SUBST([AM_CFLAGS], ["$my_am_cflags"])
Signature · 1 line
1.7.0.1
PATCH 2/24 Remove warnings from nfsctl.c
In reply to Steve Dicksonnfsctl.c: In function 'expsetup':
nfsctl.c:112: warning: signed and unsigned type in conditional expression
--- support/export/nfsctl.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
support/export/nfsctl.c +1 -1
@@ -109,7 +109,7 @@ expsetup(struct nfsctl_export *exparg, nfs_export *exp, int unexport)str_tolower (exparg->ex_client); exparg->ex_flags = exp->m_export.e_flags; exparg->ex_dev = (!unexport && (exp->m_export.e_flags &NFSEXP_FSID )) ? - exp->m_export.e_fsid : stb.st_dev; + (__nfsd_dev_t )exp->m_export.e_fsid : stb.st_dev; exparg->ex_ino = stb.st_ino; exparg->ex_anon_uid = exp->m_export.e_anonuid; exparg->ex_anon_gid = exp->m_export.e_anongid;
Signature · 1 line
1.7.0.1
Re: [PATCH 02/24] Remove warnings from nfsctl.c
Quoted 13 lines · Steve Dickson
nfsctl.c: In function 'expsetup':
nfsctl.c:112: warning: signed and unsigned type in conditional expressionSigned-off-by Steve Dickson <[email protected]>--- support/export/nfsctl.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)support/export/nfsctl.c +0
-0
support/export/nfsctl.c expsetup(struct nfsctl_export *exparg, nfs_export *exp, int unexport) · line 112
Quoted 5 lines
support/export/nfsctl.c +1
-1@@ -109,7 +109,7 @@ expsetup(struct nfsctl_export *exparg, nfs_export *exp, int unexport)str_tolower (exparg->ex_client); exparg->ex_flags = exp->m_export.e_flags; exparg->ex_dev = (!unexport && (exp->m_export.e_flags &NFSEXP_FSID )) ? - exp->m_export.e_fsid : stb.st_dev; + (__nfsd_dev_t )exp->m_export.e_fsid : stb.st_dev;
Any reason not to just define e_fsid as an __nfsd_dev_t to start off with?
--b.
Quoted 10 lines
support/export/nfsctl.c +0
-0exparg->ex_ino = stb.st_ino; exparg->ex_anon_uid = exp->m_export.e_anonuid; exparg->ex_anon_gid = exp->m_export.e_anongid;Signature · 6 lines
1.7.0.1 -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 02/24] Remove warnings from nfsctl.c
Quoted 9 lines · J. Bruce Fields
Quoted 8 lines · Steve Dickson
nfsctl.c: In function 'expsetup':
nfsctl.c:112: warning: signed and unsigned type in conditional expressionSigned-off-by Steve Dickson <[email protected]>--- support/export/nfsctl.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
Quoted 13 lines
support/export/nfsctl.c expsetup(struct nfsctl_export *exparg, nfs_export *exp, int unexport) · line 112
Quoted 10 lines
support/export/nfsctl.c +1
-1@@ -109,7 +109,7 @@ expsetup(struct nfsctl_export *exparg, nfs_export *exp, int unexport)str_tolower (exparg->ex_client); exparg->ex_flags = exp->m_export.e_flags; exparg->ex_dev = (!unexport && (exp->m_export.e_flags &NFSEXP_FSID )) ? - exp->m_export.e_fsid : stb.st_dev; + (__nfsd_dev_t )exp->m_export.e_fsid : stb.st_dev;Any reason not to just define
e_fsidas an__nfsd_dev_tto start off with?
Well would changing e_fsid from a int to an unsigned long (i.e. __nfsd_dev_t) break the dump_to_cache() interface between the mound and the kernel?
steved.
Quoted 17 lines
--b.
Quoted 10 lines
support/export/nfsctl.c +0
-0exparg->ex_ino = stb.st_ino; exparg->ex_anon_uid = exp->m_export.e_anonuid; exparg->ex_anon_gid = exp->m_export.e_anongid;Signature · 6 lines
1.7.0.1 -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 3/24 Removed warnings from rpcdispatch.c
In reply to Steve Dicksonrpcdispatch.c: In function 'rpc_dispatch':
rpcdispatch.c:30: warning: comparison between signed and unsigned
integer expressions
rpcdispatch.c:35: warning: comparison between signed and unsigned
integer expressions
--- support/nfs/rpcdispatch.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
support/nfs/rpcdispatch.c +2 -2
@@ -27,12 +27,12 @@ rpc_dispatch(struct svc_req *rqstp, SVCXPRT *transp, {struct rpc_dentry *dent; -if (rqstp->rq_vers > nvers) { +if (((int )rqstp->rq_vers) > nvers) {svcerr_progvers (transp,1 , nvers);return ; } dtable += (rqstp->rq_vers -1 ); -if (rqstp->rq_proc > dtable->nproc) { +if (((int )rqstp->rq_proc) > dtable->nproc) {svcerr_noproc (transp);return ; }
Signature · 1 line
1.7.0.1
PATCH 4/24 Remove warnings from svc_socket.c
In reply to Steve Dicksonsvc_socket.c: In function 'svcudp_socket':
svc_socket.c:160: warning: unused parameter 'reuse'
--- support/include/nfslib.h | 2 +- support/nfs/rpcmisc.c | 2 +- support/nfs/svc_socket.c | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-)
support/include/nfslib.h +1 -1
@@ -158,6 +158,6 @@ void qword_printuint(FILE *f, unsigned int num);void closeall (int min);int svctcp_socket (u_long __number,int __reuse); -int svcudp_socket (u_long __number,int __reuse); +int svcudp_socket (u_long __number);#endif /* NFSLIB_H */
support/nfs/rpcmisc.c +1 -1
@@ -154,7 +154,7 @@ rpc_init(char *name, int prog, int vers, sock =makesock (defport,IPPROTO_UDP ); }if (sock ==RPC_ANYSOCK ) - sock =svcudp_socket (prog,1 ); + sock =svcudp_socket (prog); transp =svcudp_create (sock);if (transp ==NULL ) {xlog (L_FATAL ,"cannot create udp service." );
support/nfs/svc_socket.c +3 -3
@@ -157,9 +157,9 @@ svctcp_socket (u_long number, int reuse)* Create and bind a UDP socket based on program number */ int -svcudp_socket (u_long number,int reuse) +svcudp_socket (u_long number) { -return svc_socket (number,SOCK_DGRAM ,IPPROTO_UDP ,0 ); +return svc_socket (number,SOCK_DGRAM ,IPPROTO_UDP , FALSE); }#ifdef TEST
@@ -174,7 +174,7 @@ check (u_long number, u_short port, int protocol, int reuse)if (protocol ==IPPROTO_TCP ) socket =svctcp_socket (number, reuse);else - socket =svcudp_socket (number, reuse); + socket =svcudp_socket (number);if (socket <0 )return 1 ;
Signature · 1 line
1.7.0.1
PATCH 5/24 emove warnings from cacheio.c
In reply to Steve Dicksoncacheio.c: In function 'cache_flush':
cacheio.c:352: warning: comparison between signed and unsigned integer expressions
--- support/nfs/cacheio.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
support/nfs/cacheio.c +1 -1
@@ -349,7 +349,7 @@ cache_flush(int force)sprintf (path,"/proc/net/rpc/%s/flush" , cachelist[c]); fd =open (path,O_RDWR );if (fd >=0 ) { -if (write (fd, stime,strlen (stime)) !=strlen (stime)) { +if (write (fd, stime,strlen (stime)) != (ssize_t )strlen (stime)) {xlog_warn ("Writing to '%s' failed: errno %d (%s)" , path, errno,strerror (errno)); }
Signature · 1 line
1.7.0.1
PATCH 6/24 Remove warnings from nfs_mntent.c
In reply to Steve Dicksonnfs_mntent.c: In function 'mangle':
nfs_mntent.c:36: warning: comparison between signed and unsigned integer expressions
--- support/nfs/nfs_mntent.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
support/nfs/nfs_mntent.c +1 -1
@@ -28,7 +28,7 @@ static char * mangle(const char *arg) {const unsigned char *s = (const unsigned char *)arg;char *ss, *sp; -int n; +unsigned int n; n =strlen (arg); ss = sp =xmalloc (4 *n+1 );
Signature · 1 line
1.7.0.1
PATCH 7/24 Removed warnings from conffile.c
In reply to Steve Dicksonconffile.c: In function 'conf_parse_line':
conffile.c:301: warning: comparison between signed and unsigned integer expressions
conffile.c: In function 'conf_load_defaults':
conffile.c:356: warning: unused parameter 'tr'
--- support/nfs/conffile.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-)
support/nfs/conffile.c +4 -4
@@ -49,7 +49,7 @@#include "conffile.h" #include "xlog.h" -static void conf_load_defaults (int ); +static void conf_load_defaults (void );static int conf_set (int ,char *,char *,char *,char *,int ,int );
@@ -212,7 +212,7 @@ conf_parse_line(int trans, char *line, size_t sz) {char *val, *ptr;size_t i; -int j; +size_t j;static char *section =0 ;static char *arg =0 ;static int ln =0 ;
@@ -353,7 +353,7 @@ conf_parse(int trans, char *buf, size_t sz) }static void -conf_load_defaults (int tr) +conf_load_defaults (void ) {/* No defaults */ return ;
@@ -412,7 +412,7 @@ conf_reinit(void) trans =conf_begin ();/* Load default configuration values. */ -conf_load_defaults (trans); +conf_load_defaults ();/* Free potential existing configuration. */ if (conf_addr) {
Signature · 1 line
1.7.0.1
PATCH 8/24 Removed warnings from mountd.c
In reply to Steve Dicksonmountd.c: In function 'mount_null_1_svc':
mountd.c:195: warning: unused parameter 'rqstp' mountd.c:195: warning: unused parameter 'argp' mountd.c:195: warning: unused parameter 'resp' mountd.c: In function 'mount_dump_1_svc':
mountd.c:213: warning: unused parameter 'argp' mountd.c: In function 'mount_umnt_1_svc':
mountd.c:224: warning: unused parameter 'resp' mountd.c: In function 'mount_umntall_1_svc':
mountd.c:248: warning: unused parameter 'argp' mountd.c:248: warning: unused parameter 'resp' mountd.c: In function 'mount_export_1_svc':
mountd.c:258: warning: unused parameter 'argp' mountd.c: In function 'mount_exportall_1_svc':
mountd.c:269: warning: unused parameter 'argp' mountd.c: In function 'mount_dump_1_svc':
mountd.c:216: warning: unused parameter 'argp' mountd.c: In function 'mount_umnt_1_svc':
mountd.c:227: warning: unused parameter 'resp' mountd.c: In function 'mount_umntall_1_svc':
mountd.c:251: warning: unused parameter 'argp' mountd.c:251: warning: unused parameter 'resp' mountd.c: In function 'mount_export_1_svc':
mountd.c:261: warning: unused parameter 'argp' mountd.c: In function 'mount_exportall_1_svc':
mountd.c:272: warning: unused parameter 'argp'
--- utils/mountd/mountd.c | 19 +++++++++++++++---- 1 files changed, 15 insertions(+), 4 deletions(-)
utils/mountd/mountd.c +15 -4
@@ -194,6 +194,9 @@ sig_hup (int sig)bool_t mount_null_1_svc (struct svc_req *rqstp,void *argp,void *resp) { +xlog (D_CALL ,"MNT1(%s) null: rqstp %p argp %p resp %p" , + rqstp, argp, resp); +return 1 ; }
@@ -214,7 +217,8 @@ mount_dump_1_svc(struct svc_req *rqstp, void *argp, mountlist *res) {struct sockaddr_in *addr =nfs_getrpccaller_in (rqstp->rq_xprt); -xlog (D_CALL ,"dump request from %s." ,inet_ntoa (addr->sin_addr)); +xlog (D_CALL ,"MNT1(%s) dump request: argp %p" , +inet_ntoa (addr->sin_addr), argp); *res =mountlist_list ();return 1 ;
@@ -235,6 +239,8 @@ mount_umnt_1_svc(struct svc_req *rqstp, dirpath *argp, void *resp) rpath[sizeof (rpath) -1 ] ='\0' ; p = rpath; } +xlog (D_CALL ,"MNT1(%s) unmount: path %s resp %p" , +inet_ntoa (sin->sin_addr), p, resp);if (!(exp =auth_authenticate ("unmount" , sin, p))) {return 1 ;
@@ -247,10 +253,15 @@ mount_umnt_1_svc(struct svc_req *rqstp, dirpath *argp, void *resp)bool_t mount_umntall_1_svc (struct svc_req *rqstp,void *argp,void *resp) { +struct sockaddr_in *sin =nfs_getrpccaller_in (rqstp->rq_xprt); + +xlog (D_CALL ,"UMNT1(%s) unmountall: argp %s resp %p" , +inet_ntoa (sin->sin_addr), argp, resp); +/* Reload /etc/xtab if necessary */ auth_reload (); -mountlist_del_all (nfs_getrpccaller_in (rqstp->rq_xprt)); +mountlist_del_all (sin);return 1 ; }
@@ -259,7 +270,7 @@ mount_export_1_svc(struct svc_req *rqstp, void *argp, exports *resp) {struct sockaddr_in *addr =nfs_getrpccaller_in (rqstp->rq_xprt); -xlog (D_CALL ,"export request from %s." ,inet_ntoa (addr->sin_addr)); +xlog (D_CALL ,"EXPORT(%s) dump: argp %p" ,inet_ntoa (addr->sin_addr), argp); *resp =get_exportlist ();return 1 ;
@@ -270,7 +281,7 @@ mount_exportall_1_svc(struct svc_req *rqstp, void *argp, exports *resp) {struct sockaddr_in *addr =nfs_getrpccaller_in (rqstp->rq_xprt); -xlog (D_CALL ,"exportall request from %s." ,inet_ntoa (addr->sin_addr)); +xlog (D_CALL ,"EXPORTALL(%s) dump: argp %p" ,inet_ntoa (addr->sin_addr), argp); *resp =get_exportlist ();return 1 ;
Signature · 1 line
1.7.0.1
Re: [PATCH 08/24] Removed warnings from mountd.c
Quoted 6 lines · Steve Dickson
mountd.c: In function 'mount_null_1_svc':
mountd.c:195: warning: unused parameter 'rqstp'
mountd.c:195: warning: unused parameter 'argp'
mountd.c:195: warning: unused parameter 'resp'mountd.c: In function 'mount_dump_1_svc':
mountd.c:213: warning: unused parameter 'argp'
...
utils/mountd/mountd.c mount_dump_1_svc(struct svc_req *rqstp, void *argp, mountlist *res) · line 221
Quoted 7 lines
utils/mountd/mountd.c +2
-1@@ -214,7 +217,8 @@ mount_dump_1_svc(struct svc_req *rqstp, void *argp, mountlist *res) {struct sockaddr_in *addr =nfs_getrpccaller_in (rqstp->rq_xprt); -xlog (D_CALL ,"dump request from %s." ,inet_ntoa (addr->sin_addr)); +xlog (D_CALL ,"MNT1(%s) dump request: argp %p" , +inet_ntoa (addr->sin_addr), argp);
Since those are unused, adding them to the debugging log doesn't add any useful information.
The extra warnings is maybe a useful exercise to do every now and then to check if it turns up bugs, but I'm not sure forcing all the code to pass those tests is worthwhile.
--b.
Re: [PATCH 08/24] Removed warnings from mountd.c
Quoted 12 lines · J. Bruce Fields
Quoted 6 lines · Steve Dickson
mountd.c: In function 'mount_null_1_svc':
mountd.c:195: warning: unused parameter 'rqstp'
mountd.c:195: warning: unused parameter 'argp'
mountd.c:195: warning: unused parameter 'resp'mountd.c: In function 'mount_dump_1_svc':
mountd.c:213: warning: unused parameter 'argp'...
Quoted 4 lines
utils/mountd/mountd.c +0
-0@@ -214,7 +217,8 @@ mount_dump_1_svc(struct svc_req *rqstp, void *argp, mountlist *res) {struct sockaddr_in *addr =nfs_getrpccaller_in (rqstp->rq_xprt);
Quoted 6 lines
utils/mountd/mountd.c mount_dump_1_svc(struct svc_req *rqstp, void *argp, mountlist *res) · line 221
Quoted 3 lines
utils/mountd/mountd.c +2
-1-xlog (D_CALL ,"dump request from %s." ,inet_ntoa (addr->sin_addr)); +xlog (D_CALL ,"MNT1(%s) dump request: argp %p" , +inet_ntoa (addr->sin_addr), argp);Since those are unused, adding them to the debugging log doesn't add any useful information.
Well knowing that a client just sent a dump request when debug is enabled verses not knowing with debug enabled seems it might be somewhat useful...
Quoted 4 lines
The extra warnings is maybe a useful exercise to do every now and then to check if it turns up bugs, but I'm not sure forcing all the code to pass those tests is worthwhile.
Really... Maybe I'm misunderstanding you but forcing all the code to compile warning free is a really good thing...
steved.
Re: [PATCH 08/24] Removed warnings from mountd.c
In reply to Steve DicksonHi Steve-
Quoted 57 lines · Steve Dickson
mountd.c: In function 'mount_null_1_svc':
mountd.c:195: warning: unused parameter 'rqstp' mountd.c:195: warning: unused parameter 'argp' mountd.c:195: warning: unused parameter 'resp'mountd.c: In function 'mount_dump_1_svc':
mountd.c:213: warning: unused parameter 'argp'mountd.c: In function 'mount_umnt_1_svc':
mountd.c:224: warning: unused parameter 'resp'mountd.c: In function 'mount_umntall_1_svc':
mountd.c:248: warning: unused parameter 'argp' mountd.c:248: warning: unused parameter 'resp'mountd.c: In function 'mount_export_1_svc':
mountd.c:258: warning: unused parameter 'argp'mountd.c: In function 'mount_exportall_1_svc':
mountd.c:269: warning: unused parameter 'argp'mountd.c: In function 'mount_dump_1_svc':
mountd.c:216: warning: unused parameter 'argp'mountd.c: In function 'mount_umnt_1_svc':
mountd.c:227: warning: unused parameter 'resp'mountd.c: In function 'mount_umntall_1_svc':
mountd.c:251: warning: unused parameter 'argp' mountd.c:251: warning: unused parameter 'resp'mountd.c: In function 'mount_export_1_svc':
mountd.c:261: warning: unused parameter 'argp'mountd.c: In function 'mount_exportall_1_svc':
mountd.c:272: warning: unused parameter 'argp'Signed-off-by Steve Dickson<[email protected]>--- utils/mountd/mountd.c | 19 +++++++++++++++---- 1 files changed, 15 insertions(+), 4 deletions(-)utils/mountd/mountd.c +5
-1@@ -194,6 +194,9 @@ sig_hup (int sig)bool_t mount_null_1_svc (struct svc_req *rqstp,void *argp,void *resp) { +xlog (D_CALL ,"MNT1(%s) null: rqstp %p argp %p resp %p" , + rqstp, argp, resp); +return 1 ; }@@ -214,7 +217,8 @@ mount_dump_1_svc(struct svc_req *rqstp, void *argp, mountlist *res) {struct sockaddr_in *addr =nfs_getrpccaller_in (rqstp->rq_xprt); -xlog (D_CALL ,"dump request from %s." ,inet_ntoa (addr->sin_addr)); +xlog (D_CALL ,"MNT1(%s) dump request: argp %p" , +inet_ntoa (addr->sin_addr), argp); *res =mountlist_list ();return 1 ;
utils/mountd/mountd.c mount_umnt_1_svc(struct svc_req *rqstp, dirpath *argp, void *resp) · line 243
Quoted 5 lines
utils/mountd/mountd.c +2
-0@@ -235,6 +239,8 @@ mount_umnt_1_svc(struct svc_req *rqstp, dirpath *argp, void *resp) rpath[sizeof (rpath) -1 ] ='\0' ; p = rpath; } +xlog (D_CALL ,"MNT1(%s) unmount: path %s resp %p" , +inet_ntoa (sin->sin_addr), p, resp);
I have to agree with Bruce: adding xlog() calls here is the wrong thing to do. Instead, use
__attribute__((unused))
for the unused parameters.
Quoted 6 lines
utils/mountd/mountd.c +0
-0if (!(exp =auth_authenticate ("unmount" , sin, p))) {return 1 ;@@ -247,10 +253,15 @@ mount_umnt_1_svc(struct svc_req *rqstp, dirpath *argp, void *resp)bool_t mount_umntall_1_svc (struct svc_req *rqstp,void *argp,void *resp) {
utils/mountd/mountd.c mount_umnt_1_svc(struct svc_req *rqstp, dirpath *argp, void *resp) · line 264
Quoted 10 lines
utils/mountd/mountd.c +6
-1+struct sockaddr_in *sin =nfs_getrpccaller_in (rqstp->rq_xprt); + +xlog (D_CALL ,"UMNT1(%s) unmountall: argp %s resp %p" , +inet_ntoa (sin->sin_addr), argp, resp); +/* Reload /etc/xtab if necessary */ auth_reload (); -mountlist_del_all (nfs_getrpccaller_in (rqstp->rq_xprt)); +mountlist_del_all (sin);
You couldn't wait until the rest of my IPv6 mountd patches are integrated? I address a lot of these problems in those patches, and now I'm going to have to rebase all of that work. What's the rush?
Quoted 21 lines
utils/mountd/mountd.c +2
-2return 1 ; }@@ -259,7 +270,7 @@ mount_export_1_svc(struct svc_req *rqstp, void *argp, exports *resp) {struct sockaddr_in *addr =nfs_getrpccaller_in (rqstp->rq_xprt); -xlog (D_CALL ,"export request from %s." ,inet_ntoa (addr->sin_addr)); +xlog (D_CALL ,"EXPORT(%s) dump: argp %p" ,inet_ntoa (addr->sin_addr), argp); *resp =get_exportlist ();return 1 ;@@ -270,7 +281,7 @@ mount_exportall_1_svc(struct svc_req *rqstp, void *argp, exports *resp) {struct sockaddr_in *addr =nfs_getrpccaller_in (rqstp->rq_xprt); -xlog (D_CALL ,"exportall request from %s." ,inet_ntoa (addr->sin_addr)); +xlog (D_CALL ,"EXPORTALL(%s) dump: argp %p" ,inet_ntoa (addr->sin_addr), argp); *resp =get_exportlist ();return 1 ;
Re: [PATCH 08/24] Removed warnings from mountd.c
Quoted 21 lines · Chuck Lever
Hi Steve-
Quoted 6 lines
utils/mountd/mountd.c +0
-0utils/mountd/mountd.c sig_hup (int sig) · line 202
Quoted 9 lines
utils/mountd/mountd.c +3
-0@@ -194,6 +194,9 @@ sig_hup (int sig)bool_t mount_null_1_svc (struct svc_req *rqstp,void *argp,void *resp) { +xlog (D_CALL ,"MNT1(%s) null: rqstp %p argp %p resp %p" , + rqstp, argp, resp); +return 1 ; }I have to agree with Bruce: adding
xlog()calls here is the wrong thing to do. Instead, use
Quoted 3 lines
__attribute__((unused))for the unused parameters.
Boy... using those really make the code look ugly... bool_t
mount_null_1_svc(
__attribute__((unused))struct svc_req *rqstp ,
__attribute__((unused))void *argp,
__attribute__((unused))void *resp)
{
return 1;
}So ugly up the code verses adding a bit more debugging... I must say I'm leaning toward the latter...
Quoted 16 lines
Quoted 16 lines
if (!(exp = auth_authenticate("unmount", sin, p))) { return 1;utils/mountd/mountd.c +5
-0@@ -247,10 +253,15 @@ mount_umnt_1_svc(struct svc_req *rqstp, dirpath *argp, void *resp)bool_t mount_umntall_1_svc (struct svc_req *rqstp,void *argp,void *resp) { +struct sockaddr_in *sin =nfs_getrpccaller_in (rqstp->rq_xprt); + +xlog (D_CALL ,"UMNT1(%s) unmountall: argp %s resp %p" , +inet_ntoa (sin->sin_addr), argp, resp); +/* Reload /etc/xtab if necessary */ auth_reload ();
Quoted 6 lines
utils/mountd/mountd.c mount_umnt_1_svc(struct svc_req *rqstp, dirpath *argp, void *resp) · line 264
Quoted 2 lines
utils/mountd/mountd.c +1
-1-mountlist_del_all (nfs_getrpccaller_in (rqstp->rq_xprt)); +mountlist_del_all (sin);You couldn't wait until the rest of my IPv6 mountd patches are integrated? I address a lot of these problems in those patches, and now I'm going to have to rebase all of that work. What's the rush?
No rush.. I just wanted to clean things up...
steved.
Re: [PATCH 08/24] Removed warnings from mountd.c
Quoted 33 lines · Steve Dickson
Quoted 21 lines · Chuck Lever
Hi Steve-
Quoted 6 lines
utils/mountd/mountd.c +0
-0utils/mountd/mountd.c sig_hup (int sig) · line 202
Quoted 9 lines
utils/mountd/mountd.c +3
-0@@ -194,6 +194,9 @@ sig_hup (int sig)bool_t mount_null_1_svc (struct svc_req *rqstp,void *argp,void *resp) { +xlog (D_CALL ,"MNT1(%s) null: rqstp %p argp %p resp %p" , + rqstp, argp, resp); +return 1 ; }I have to agree with Bruce: adding
xlog()calls here is the wrong thing to do. Instead, useQuoted 3 lines
__attribute__((unused))
for the unused parameters.
Boy... using those really make the code look ugly...
bool_t
mount_null_1_svc(
__attribute__((unused))structsvc_req*rqstp , __attribute__((unused))void *argp, __attribute__((unused))void *resp)
Quoted 5 lines
{ return 1; }So ugly up the code verses adding a bit more debugging... I must say I'm leaning toward the latter...
Aesthetics are in the eye of the beholder. I don't find that at all difficult to read. In fact, listing each parameter on a separate line is perhaps somewhat nicer that what we have here, and has a long history in C coding style. If the look really bugs you, you can do what so many software projects before us have done:
#define ARG_UNUSED __attribute__((unused))
or even:
#define _unused_ __attribute__((unused))
I seem to recall that /usr/include/ldap.h has an example of this. We may already have something like this in one of the nfs-utils headers. There could also be shorter synonyms for __attribute__((unused)) available from the compiler.
Debugging messages in general are fine, but really there's no need to show the pointer addresses of the arguments and response for a NULL procedure, for example; that's entirely useless for debugging. And, like the __attribute__ directive, adding noise in these messages can also be viewed as needless visual clutter (both in the source code, and in the system log, making it somewhat of a double sin).
A useful debugging message would display just the name of the procedure (eg. MNTPROC_NULL) and the client's IP address, and any arguments (NULL has none). I'm not objecting to adding a debugging message here, but adding it solely for the purpose of quelling a compiler warning is, well, silly. Let's fix the compiler warnings with an appropriate direct remedy, then consider introducing a debugging message on its own merit.
Quoted 15 lines
Quoted 15 lines
Quoted 15 lines
if (!(exp = auth_authenticate("unmount", sin, p))) { return 1;utils/mountd/mountd.c +5
-0@@ -247,10 +253,15 @@ mount_umnt_1_svc(struct svc_req *rqstp, dirpath *argp, void *resp)bool_t mount_umntall_1_svc (struct svc_req *rqstp,void *argp,void *resp) { +struct sockaddr_in *sin =nfs_getrpccaller_in (rqstp->rq_xprt); + +xlog (D_CALL ,"UMNT1(%s) unmountall: argp %s resp %p" , +inet_ntoa (sin->sin_addr), argp, resp); +/* Reload /etc/xtab if necessary */ auth_reload ();
Quoted 7 lines
Quoted 6 lines
utils/mountd/mountd.c mount_umnt_1_svc(struct svc_req *rqstp, dirpath *argp, void *resp) · line 264
Quoted 2 lines
utils/mountd/mountd.c +1
-1-mountlist_del_all (nfs_getrpccaller_in (rqstp->rq_xprt)); +mountlist_del_all (sin);You couldn't wait until the rest of my IPv6 mountd patches are integrated? I address a lot of these problems in those patches, and now I'm going to have to rebase all of that work. What's the rush?
No rush.. I just wanted to clean things up...
Cleaning is good :-) I've got many similar changes in my tree, and I'm committed to not introducing any new compiler warnings in my nfs-utils submissions.
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
PATCH 9/24 Removed warnings from fsloc.c
In reply to Steve Dicksonfsloc.c: In function 'replicas_lookup':
fsloc.c:149: warning: unused parameter 'key'
--- utils/mountd/cache.c | 2 +- utils/mountd/fsloc.c | 2 +- utils/mountd/fsloc.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)
utils/mountd/cache.c +1 -1
@@ -600,7 +600,7 @@ static void write_fsloc(FILE *f, struct exportent *ep, char *path)if (ep->e_fslocmethod ==FSLOC_NONE )return ; - servers =replicas_lookup (ep->e_fslocmethod, ep->e_fslocdata, path); + servers =replicas_lookup (ep->e_fslocmethod, ep->e_fslocdata);if (!servers)return ;qword_print (f,"fsloc" );
utils/mountd/fsloc.c +1 -1
@@ -146,7 +146,7 @@ static struct servers *method_list(char *data) }/* Returns appropriately filled struct servers, or NULL if had a problem */ -struct servers *replicas_lookup (int method,char *data,char *key) +struct servers *replicas_lookup (int method,char *data) {struct servers *sp=NULL ;switch (method) {
utils/mountd/fsloc.h +1 -1
@@ -44,7 +44,7 @@ struct servers { int h_referral; /* 0=replica, 1=referral */ }; -struct servers *replicas_lookup (int method,char *data,char *key); +struct servers *replicas_lookup (int method,char *data);void release_replicas (struct servers *server);#endif /* FSLOC_H */
Signature · 1 line
1.7.0.1
PATCH 10/24 Removed warnings from cache.c
In reply to Steve Dicksoncache.c:812: warning: missing initializer
cache.c:812: warning: (near initialization for 'cachelist[0].f') cache.c:813: warning: missing initializer
cache.c:813: warning: (near initialization for 'cachelist[1].f') cache.c:814: warning: missing initializer
cache.c:814: warning: (near initialization for 'cachelist[2].f') cache.c:815: warning: missing initializer
cache.c:815: warning: (near initialization for 'cachelist[3].f') cache.c:816: warning: missing initializer
cache.c:816: warning: (near initialization for 'cachelist[4].f') cache.c: In function 'cache_export_ent':
cache.c:887: warning: comparison between signed and unsigned integer expressions cache.c:907: warning: comparison between signed and unsigned integer expressions
--- support/include/nfslib.h | 2 +- utils/mountd/cache.c | 18 +++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-)
support/include/nfslib.h +1 -1
@@ -83,7 +83,7 @@ struct exportent {int e_nsquids;int * e_sqgids;int e_nsqgids; -int e_fsid; +unsigned int e_fsid;char * e_mountpoint;int e_fslocmethod;char * e_fslocdata;
utils/mountd/cache.c +9 -9
@@ -593,7 +593,7 @@ void nfsd_fh(FILE *f)return ; } -static void write_fsloc (FILE *f,struct exportent *ep,char *path) +static void write_fsloc (FILE *f,struct exportent *ep) {struct servers *servers;
@@ -648,7 +648,7 @@ static int dump_to_cache(FILE *f, char *domain, char *path, struct exportent *exqword_printint (f, exp->e_anonuid);qword_printint (f, exp->e_anongid);qword_printint (f, exp->e_fsid); -write_fsloc (f, exp, path); +write_fsloc (f, exp);write_secinfo (f, exp, flag_mask);if (exp->e_uuid ==NULL || different_fs) {char u[16 ];
@@ -809,11 +809,11 @@ struct {void (*cache_handle)(FILE *f);FILE *f; } cachelist[] = { - { "auth.unix.ip", auth_unix_ip}, - { "auth.unix.gid", auth_unix_gid}, - { "nfsd.export", nfsd_export}, - { "nfsd.fh", nfsd_fh}, - { NULL, NULL } + { "auth.unix.ip", auth_unix_ip, NULL}, + { "auth.unix.gid", auth_unix_gid, NULL}, + { "nfsd.export", nfsd_export, NULL}, + { "nfsd.fh", nfsd_fh, NULL}, + { NULL, NULL, NULL } }; extern int manage_gids;
@@ -881,8 +881,8 @@ int cache_export_ent(char *domain, struct exportent *exp, char *path)* and export them with the same options */ struct stat stb; -int l =strlen (exp->e_path); -int dev; +size_t l =strlen (exp->e_path); +__dev_t dev;if (strlen (path) <= l || path[l] !='/' ||strncmp (exp->e_path, path, l) !=0 )
Signature · 1 line
1.7.0.1
PATCH 11/24 Removed warnings from nfssvc.c
In reply to Steve Dicksonnfssvc.c:184: warning: comparison between signed and unsigned integer expressions nfssvc.c: In function 'nfssvc_setvers':
nfssvc.c:254: warning: comparison between signed and unsigned integer expressions nfssvc.c: In function 'nfssvc_threads':
nfssvc.c:280: warning: comparison between signed and unsigned integer expressions
--- utils/nfsd/nfssvc.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)
utils/nfsd/nfssvc.c +3 -3
@@ -181,7 +181,7 @@ nfssvc_setfds(const struct addrinfo *hints, const char *node, const char *port) }snprintf (buf,sizeof (buf),"%d\n" , sockfd); -if (write (fd, buf,strlen (buf)) !=strlen (buf)) { +if (write (fd, buf,strlen (buf)) != (ssize_t )strlen (buf)) {/* * this error may be common on older kernels that don't * support IPv6, so turn into a debug message.
@@ -251,7 +251,7 @@ nfssvc_setvers(unsigned int ctlbits, int minorvers4) }xlog (D_GENERAL ,"Writing version string to kernel: %s" , buf);snprintf (ptr+off,sizeof (buf) - off,"\n" ); -if (write (fd, buf,strlen (buf)) !=strlen (buf)) +if (write (fd, buf,strlen (buf)) != (ssize_t )strlen (buf))xlog (L_ERROR ,"Setting version failed: errno %d (%m)" , errno);close (fd);
@@ -277,7 +277,7 @@ nfssvc_threads(unsigned short port, const int nrservs)snprintf (buf,sizeof (buf),"%d\n" , nrservs); n =write (fd, buf,strlen (buf));close (fd); -if (n !=strlen (buf)) +if (n != (ssize_t )strlen (buf))return -1 ;else return 0 ;
Signature · 1 line
1.7.0.1
PATCH 12/24 Removed warnings from nfsstat.c
In reply to Steve Dicksonnfsstat.c: In function 'print_callstats':
nfsstat.c:797: warning: comparison between signed and unsigned integer expressions
nfsstat.c:801: warning: comparison between signed and unsigned integer expressions
nfsstat.c:802: warning: comparison between signed and unsigned integer expressions
nfsstat.c:805: warning: comparison between signed and unsigned integer expressions
nfsstat.c: In function 'print_callstats_list':
nfsstat.c:821: warning: comparison between signed and unsigned integer expressions
nfsstat.c:828: warning: comparison between signed and unsigned integer expressions
nfsstat.c: In function 'unpause':
nfsstat.c:1111: warning: unused parameter 'sig'
--- utils/nfsstat/nfsstat.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)
utils/nfsstat/nfsstat.c +3 -3
@@ -791,7 +791,7 @@ print_callstats(const char *hdr, const char **names, {unsigned long long total;unsigned long long pct; -int i, j; +unsigned int i, j;fputs (hdr, stdout);for (i =0 , total =0 ; i < nr; i++)
@@ -816,7 +816,7 @@ print_callstats_list(const char *hdr, const char **names, unsigned int *callinfo, unsigned int nr) {unsigned long long calltotal; -int i; +unsigned int i;for (i =0 , calltotal =0 ; i < nr; i++) { calltotal += callinfo[i];
@@ -1118,7 +1118,7 @@ unpause(int sig) time_diff =difftime (endtime, starttime); minutes = time_diff /60 ; seconds = (int )time_diff %60 ; -printf ("Signal received; displaying (only) statistics gathered over the last %d minutes, %d seconds:\n\n" , minutes, seconds); +printf ("Signal %d received; displaying (only) statistics gathered over the last %d minutes, %d seconds:\n\n" , sig, minutes, seconds); } static void
Signature · 1 line
1.7.0.1
PATCH 13/24 Removed warnings from atomicio.c
In reply to Steve Dicksonatomicio.c: In function 'atomicio':
atomicio.c:48: warning: comparison between signed and unsigned integer expressions
--- utils/idmapd/atomicio.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
utils/idmapd/atomicio.c +2 -1
@@ -43,7 +43,8 @@ atomicio( size_t n) {char *s = _s; -ssize_t res, pos =0 ; +ssize_t res; +size_t pos =0 ;while (n > pos) { res = (f) (fd, s + pos, n - pos);
Signature · 1 line
1.7.0.1
PATCH 14/24 Removed warnings from idmapd.c
In reply to Steve Dicksonidmapd.c:120: warning: missing initializer
idmapd.c:120: warning: (near initialization for 'nfsd_ic[0].ic_event') idmapd.c:121: warning: missing initializer
idmapd.c:121: warning: (near initialization for 'nfsd_ic[1].ic_event') idmapd.c: In function 'flush_nfsd_cache':
idmapd.c:173: warning: comparison between signed and unsigned integer expressions idmapd.c: In function 'dirscancb':
idmapd.c:384: warning: unused parameter 'fd'
idmapd.c:384: warning: unused parameter 'which'idmapd.c: In function 'svrreopen':
idmapd.c:468: warning: unused parameter 'fd'
idmapd.c:468: warning: unused parameter 'which'
idmapd.c:468: warning: unused parameter 'data'idmapd.c: In function 'clntscancb':
idmapd.c:474: warning: unused parameter 'fd'
idmapd.c:474: warning: unused parameter 'which'idmapd.c: In function 'nfsdcb':
idmapd.c:488: warning: unused parameter 'fd'idmapd.c: In function 'nfscb':
idmapd.c:663: warning: unused parameter 'fd'idmapd.c: In function 'validateascii':
idmapd.c:850: warning: comparison between signed and unsigned integer expressions idmapd.c:858: warning: comparison between signed and unsigned integer expressions idmapd.c: In function 'getfield':
idmapd.c:916: warning: comparison between signed and unsigned integer expressions
--- utils/idmapd/idmapd.c | 36 +++++++++++++++++++++++++++++++----- 1 files changed, 31 insertions(+), 5 deletions(-)
utils/idmapd/idmapd.c +31 -5
@@ -117,8 +117,24 @@ struct idmap_client {TAILQ_ENTRY (idmap_client) ic_next; };static struct idmap_client nfsd_ic[2 ] = { -{IC_IDNAME ,"Server" ,"" ,IC_IDNAME_CHAN ,-1 ,-1 ,0 }, -{IC_NAMEID ,"Server" ,"" ,IC_NAMEID_CHAN ,-1 ,-1 ,0 }, +{ + .ic_which =IC_IDNAME , + .ic_clid ="Server" , + .ic_id ="" , + .ic_path =IC_IDNAME_CHAN , + .ic_fd =-1 , + .ic_dirfd =-1 , + .ic_scanned =0 +}, +{ + .ic_which =IC_NAMEID , + .ic_clid ="Server" , + .ic_id ="" , + .ic_path =IC_NAMEID_CHAN , + .ic_fd =-1 , + .ic_dirfd =-1 , + .ic_scanned =0 +}, };TAILQ_HEAD (idmap_clientq, idmap_client);
@@ -170,7 +186,7 @@ flush_nfsd_cache(char *path, time_t now) fd =open (path,O_RDWR );if (fd ==-1 )return -1 ; -if (write (fd, stime,strlen (stime)) !=strlen (stime)) { +if (write (fd, stime,strlen (stime)) != (ssize_t )strlen (stime)) {errx (1 ,"Flushing nfsd cache failed: errno %d (%s)" , errno,strerror (errno)); }
@@ -389,6 +405,8 @@ dirscancb(int fd, short which, void *data)char path[PATH_MAX ];struct idmap_clientq *icq = data; +xlog (D_GENERAL ,"dirscancb: fd %d which %d data %p" , fd, which, data); + nent =scandir (pipefsdir, &ents,NULL , alphasort);if (nent ==-1 ) {xlog_warn ("dirscancb: scandir(%s): %s" , pipefsdir,strerror (errno));
@@ -467,6 +485,8 @@ out:static void svrreopen (int fd,short which,void *data) { +xlog (D_GENERAL ,"svrreopen: fd %d which %d data %p" , fd, which, data); +nfsdreopen (); }
@@ -476,6 +496,8 @@ clntscancb(int fd, short which, void *data)struct idmap_clientq *icq = data;struct idmap_client *ic; +xlog (D_GENERAL ,"clntscancb: fd %d which %d data %p" , fd, which, data); + TAILQ_FOREACH(ic, icq, ic_next)if (ic->ic_fd ==-1 &&nfsopen (ic) ==-1 ) {close (ic->ic_dirfd);
@@ -496,6 +518,8 @@ nfsdcb(int fd, short which, void *data) buf1[IDMAP_MAXMSGSZ ], authbuf[IDMAP_MAXMSGSZ ], *p;unsigned long tmp; +xlog (D_GENERAL ,"nfsdcb: fd %d which %d data %p" , fd, which, data); +if (which !=EV_READ ) goto out;
@@ -665,6 +689,8 @@ nfscb(int fd, short which, void *data)struct idmap_client *ic = data;struct idmap_msg im; +xlog (D_GENERAL ,"nfscb: fd %d which %d data %p" , fd, which, data); +if (which !=EV_READ ) goto out;
@@ -845,7 +871,7 @@ nametoidres(struct idmap_msg *im)static int validateascii (char *string,u_int32_t len) { -int i; +u_int32_t i;for (i =0 ; i < len; i++) {if (string[i] =='\0' )
@@ -901,7 +927,7 @@ static int getfield(char **bpp, char *fld, size_t fldsz) {char *bp; -u_int val, n; +int val, n;while ((bp =strsep (bpp," " )) !=NULL && bp[0 ] =='\0' ) ;
Signature · 1 line
1.7.0.1
Re: [PATCH 14/24] Removed warnings from idmapd.c
Quoted 9 lines · Steve Dickson
idmapd.c:120: warning: missing initializer
idmapd.c:120: warning: (near initialization for 'nfsd_ic[0].ic_event') idmapd.c:121: warning: missing initializer
idmapd.c:121: warning: (near initialization for 'nfsd_ic[1].ic_event')idmapd.c: In function 'flush_nfsd_cache':
idmapd.c:173: warning: comparison between signed and unsigned integer expressionsidmapd.c: In function 'dirscancb':
idmapd.c:384: warning: unused parameter 'fd'
idmapd.c:384: warning: unused parameter 'which'
...
utils/idmapd/idmapd.c dirscancb(int fd, short which, void *data) · line 409
Quoted 6 lines
utils/idmapd/idmapd.c +2
-0@@ -389,6 +405,8 @@ dirscancb(int fd, short which, void *data)char path[PATH_MAX ];struct idmap_clientq *icq = data; +xlog (D_GENERAL ,"dirscancb: fd %d which %d data %p" , fd, which, data); +
Some comment as before--let's not put information in the logs that's not useful, just to silence a gcc warning.
--b.
PATCH 15/24 Removed warnings from gssd.c
In reply to Steve Dicksongssd.c: In function 'sig_hup':
gssd.c:78: warning: unused parameter 'signal'
--- utils/gssd/gssd.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
utils/gssd/gssd.c +1 -1
@@ -78,7 +78,7 @@ void sig_hup(int signal) {/* don't exit on SIGHUP */ -printerr (1 ,"Received SIGHUP... Ignoring.\n" ); +printerr (1 ,"Received SIGHUP(%d)... Ignoring.\n" , signal);return ; }
Signature · 1 line
1.7.0.1
PATCH 16/24 Removed warnings from gssd_main_loop.c
In reply to Steve Dicksongssd_main_loop.c: In function 'dir_notify_handler':
gssd_main_loop.c:64: warning: unused parameter 'sig' gssd_main_loop.c:64: warning: unused parameter 'si' gssd_main_loop.c:64: warning: unused parameter 'data'
--- utils/gssd/gssd_main_loop.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
utils/gssd/gssd_main_loop.c +2 -0
@@ -63,6 +63,8 @@ static volatile int dir_changed = 1;static void dir_notify_handler (int sig,siginfo_t *si,void *data) { +printerr (2 ,"dir_notify_handler: sig %d si %p data %p\n" , sig, si, data); + dir_changed =1 ; }
Signature · 1 line
1.7.0.1
PATCH 17/24 Removed warnings from krb5_util.c
In reply to Steve Dicksonkrb5_util.c: In function 'realm_and_service_match':
krb5_util.c:617: warning: unused parameter 'context' krb5_util.c: In function 'limit_krb5_enctypes':
krb5_util.c:1275: warning: unused parameter 'uid'
--- utils/gssd/gssd_proc.c | 2 +- utils/gssd/krb5_util.c | 29 +++++++++++++++++++++-------- utils/gssd/krb5_util.h | 2 +- 3 files changed, 23 insertions(+), 10 deletions(-)
utils/gssd/gssd_proc.c +1 -1
@@ -859,7 +859,7 @@ int create_auth_rpc_client(struct clnt_info *clp,* Do this before creating rpc connection since we won't need * rpc connection if it fails! */ -if (limit_krb5_enctypes (&sec, uid)) { +if (limit_krb5_enctypes (&sec)) {printerr (1 ,"WARNING: Failed while limiting krb5 " "encryption types for user with uid %d\n" , uid);
utils/gssd/krb5_util.c +21 -8
@@ -613,24 +613,32 @@ out:* and has *any* instance (hostname), return 1. * Otherwise return 0, indicating no match. */ +#ifdef HAVE_KRB5 static int -realm_and_service_match (krb5_context context,krb5_principal p, -const char *realm,const char *service) +realm_and_service_match (krb5_principal p,const char *realm,const char *service) { -#ifdef HAVE_KRB5 /* Must have two components */ if (p->length !=2 )return 0 ; +if ((strlen (realm) == p->realm.length) && (strncmp (realm, p->realm.data, p->realm.length) ==0 ) && (strlen (service) == p->data[0 ].length) && (strncmp (service, p->data[0 ].data, p->data[0 ].length) ==0 ))return 1 ; + +return 0 ; +}#else +static int +realm_and_service_match (krb5_context context,krb5_principal p, +const char *realm,const char *service) +{const char *name, *inst;if (p->name.name_string.len !=2 )return 0 ; + name =krb5_principal_get_comp_string (context, p,0 ); inst =krb5_principal_get_comp_string (context, p,1 );if (name ==NULL || inst ==NULL )
@@ -638,9 +646,10 @@ realm_and_service_match(krb5_context context, krb5_principal p,if ((strcmp (realm, p->realm) ==0 ) && (strcmp (service, name) ==0 ))return 1 ; -#endif + return 0; } +#endif /* * Search the given keytab file looking for an entry with the given
@@ -662,7 +671,7 @@ gssd_search_krb5_keytab(krb5_context context, krb5_keytab kt,krb5_kt_cursor cursor;krb5_error_code code;struct gssd_k5_kt_princ *ple; -int retval =-1 ; +int retval =-1 , status;char kt_name[BUFSIZ ];char *pname;char *k5err =NULL ;
@@ -705,8 +714,12 @@ gssd_search_krb5_keytab(krb5_context context, krb5_keytab kt,printerr (4 ,"Processing keytab entry for principal '%s'\n" , pname);/* Use the first matching keytab entry found */ -if ((realm_and_service_match (context, kte->principal, realm, - service))) { +#ifdef HAVE_KRB5 + status =realm_and_service_match (kte->principal, realm, service); +#else + status =realm_and_service_match (context, kte->principal, realm, service); +#endif +if (status) {printerr (4 ,"We WILL use this entry (%s)\n" , pname); ple =get_ple_by_princ (context, kte->principal);/*
@@ -1272,7 +1285,7 @@ gssd_k5_get_default_realm(char **def_realm)*/ int -limit_krb5_enctypes (struct rpc_gss_sec *sec,uid_t uid) +limit_krb5_enctypes (struct rpc_gss_sec *sec) {u_int maj_stat, min_stat;gss_cred_id_t credh;
utils/gssd/krb5_util.h +1 -1
@@ -36,7 +36,7 @@ char *gssd_k5_err_msg(krb5_context context, krb5_error_code code);void gssd_k5_get_default_realm (char **def_realm);#ifdef HAVE_SET_ALLOWABLE_ENCTYPES -int limit_krb5_enctypes (struct rpc_gss_sec *sec,uid_t uid); +int limit_krb5_enctypes (struct rpc_gss_sec *sec);#endif /*
Signature · 1 line
1.7.0.1
PATCH 18/24 Removed warnings from gssd_proc.c
In reply to Steve Dicksongssd_proc.c: In function 'create_auth_rpc_client':
gssd_proc.c:939: warning: comparison between signed and unsigned integer expressions gssd_proc.c:939: warning: comparison between signed and unsigned integer expressions gssd_proc.c: In function 'handle_krb5_upcall':
gssd_proc.c:1164: warning: comparison between signed and unsigned integer expressions gssd_proc.c: In function 'handle_spkm3_upcall':
gssd_proc.c:1178: warning: comparison between signed and unsigned integer expressions
--- utils/gssd/gssd_proc.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)
utils/gssd/gssd_proc.c +3 -3
@@ -936,7 +936,7 @@ int create_auth_rpc_client(struct clnt_info *clp,if (sec.cred !=GSS_C_NO_CREDENTIAL )gss_release_cred (&min_stat, &sec.cred);/* Restore euid to original value */ -if ((save_uid !=-1 ) && (setfsuid (save_uid) != uid)) { +if (((int )save_uid !=-1 ) && (setfsuid (save_uid) != (int )uid)) {printerr (0 ,"WARNING: Failed to restore fsuid" " to uid %d from %d\n" , save_uid, uid); }
@@ -1161,7 +1161,7 @@ handle_krb5_upcall(struct clnt_info *clp) {uid_t uid; -if (read (clp->krb5_fd, &uid,sizeof (uid)) <sizeof (uid)) { +if (read (clp->krb5_fd, &uid,sizeof (uid)) < (ssize_t )sizeof (uid)) {printerr (0 ,"WARNING: failed reading uid from krb5 " "upcall pipe: %s\n" ,strerror (errno));return ;
@@ -1175,7 +1175,7 @@ handle_spkm3_upcall(struct clnt_info *clp) {uid_t uid; -if (read (clp->spkm3_fd, &uid,sizeof (uid)) <sizeof (uid)) { +if (read (clp->spkm3_fd, &uid,sizeof (uid)) < (ssize_t )sizeof (uid)) {printerr (0 ,"WARNING: failed reading uid from spkm3 " "upcall pipe: %s\n" ,strerror (errno));return ;
Signature · 1 line
1.7.0.1
PATCH 19/24 Removed warnings from svcgssd.c
In reply to Steve Dicksonsvcgssd.c: In function 'sig_hup':
svcgssd.c:160: warning: unused parameter 'signal'
--- utils/gssd/svcgssd.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
utils/gssd/svcgssd.c +1 -1
@@ -160,7 +160,7 @@ void sig_hup(int signal) {/* don't exit on SIGHUP */ -printerr (1 ,"Received SIGHUP... Ignoring.\n" ); +printerr (1 ,"Received SIGHUP(%d)... Ignoring.\n" , signal);return ; }
Signature · 1 line
1.7.0.1
PATCH 20/24 Removed warnings from svcgssd_proc.c
In reply to Steve Dicksonsvcgssd_proc.c: In function 'send_response':
svcgssd_proc.c:135: warning: unused parameter 'f'svcgssd_proc.c: In function 'handle_nullreq':
svcgssd_proc.c:434: warning: comparison of unsigned expression < 0 is always false
--- utils/gssd/svcgssd_proc.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-)
utils/gssd/svcgssd_proc.c +4 -4
@@ -132,7 +132,7 @@ struct gss_verifier {#define RPCSEC_GSS_SEQ_WIN 5static int -send_response (FILE *f,gss_buffer_desc *in_handle,gss_buffer_desc *in_token, +send_response (gss_buffer_desc *in_handle,gss_buffer_desc *in_token,u_int32_t maj_stat,u_int32_t min_stat,gss_buffer_desc *out_handle,gss_buffer_desc *out_token) {
@@ -431,7 +431,7 @@ handle_nullreq(FILE *f) {print_hexl ("in_tok" , in_tok.value, in_tok.length);#endif -if (in_tok.length <0 ) { +if (in_tok.length ==0 ) {printerr (0 ,"WARNING: handle_nullreq: " "failed parsing request\n" ); goto out_err;
@@ -498,7 +498,7 @@ handle_nullreq(FILE *f) {do_svc_downcall (&out_handle, &cred, mech, &ctx_token, ctx_endtime, hostbased_name); continue_needed: -send_response (f, &in_handle, &in_tok, maj_stat, min_stat, +send_response (&in_handle, &in_tok, maj_stat, min_stat, &out_handle, &out_tok); out:if (ctx_token.value !=NULL )
@@ -514,7 +514,7 @@ out: out_err:if (ctx !=GSS_C_NO_CONTEXT )gss_delete_sec_context (&ignore_min_stat, &ctx, &ignore_out_tok); -send_response (f, &in_handle, &in_tok, maj_stat, min_stat, +send_response (&in_handle, &in_tok, maj_stat, min_stat, &null_token, &null_token); goto out; }
Signature · 1 line
1.7.0.1
Re: [PATCH 20/24] Removed warnings from svcgssd_proc.c
Quoted 24 lines · Steve Dickson
svcgssd_proc.c: In function 'send_response':
svcgssd_proc.c:135: warning: unused parameter 'f'svcgssd_proc.c: In function 'handle_nullreq':
svcgssd_proc.c:434: warning: comparison of unsigned expression < 0 is always falseSigned-off-by Steve Dickson <[email protected]>--- utils/gssd/svcgssd_proc.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-)utils/gssd/svcgssd_proc.c +1
-1@@ -132,7 +132,7 @@ struct gss_verifier {#define RPCSEC_GSS_SEQ_WIN 5static int -send_response (FILE *f,gss_buffer_desc *in_handle,gss_buffer_desc *in_token, +send_response (gss_buffer_desc *in_handle,gss_buffer_desc *in_token,u_int32_t maj_stat,u_int32_t min_stat,gss_buffer_desc *out_handle,gss_buffer_desc *out_token) {
utils/gssd/svcgssd_proc.c handle_nullreq(FILE *f) { · line 434
Quoted 5 lines
utils/gssd/svcgssd_proc.c +1
-1@@ -431,7 +431,7 @@ handle_nullreq(FILE *f) {print_hexl ("in_tok" , in_tok.value, in_tok.length);#endif -if (in_tok.length <0 ) { +if (in_tok.length ==0 ) {
Is a zero-length token really illegal? I'd just ditch this check entirely.
--b.
Quoted 28 lines
utils/gssd/svcgssd_proc.c +2
-2printerr (0 ,"WARNING: handle_nullreq: " "failed parsing request\n" ); goto out_err;@@ -498,7 +498,7 @@ handle_nullreq(FILE *f) {do_svc_downcall (&out_handle, &cred, mech, &ctx_token, ctx_endtime, hostbased_name); continue_needed: -send_response (f, &in_handle, &in_tok, maj_stat, min_stat, +send_response (&in_handle, &in_tok, maj_stat, min_stat, &out_handle, &out_tok); out:if (ctx_token.value !=NULL )@@ -514,7 +514,7 @@ out: out_err:if (ctx !=GSS_C_NO_CONTEXT )gss_delete_sec_context (&ignore_min_stat, &ctx, &ignore_out_tok); -send_response (f, &in_handle, &in_tok, maj_stat, min_stat, +send_response (&in_handle, &in_tok, maj_stat, min_stat, &null_token, &null_token); goto out; }Signature · 6 lines
1.7.0.1 -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 21/24 Removed warnings from network.c
In reply to Steve Dicksonnetwork.c: In function 'nfs_verify_family':
network.c:1366: warning: unused parameter 'family'
--- utils/mount/network.c | 19 +++++++++---------- 1 files changed, 9 insertions(+), 10 deletions(-)
utils/mount/network.c +9 -10
@@ -1362,11 +1362,6 @@ nfs_nfs_port(struct mount_options *options, unsigned long *port)#ifdef IPV6_SUPPORTED sa_family_t config_default_family =AF_UNSPEC ; -static int -nfs_verify_family (sa_family_t family) -{ -return 1 ; -}#else /* IPV6_SUPPORTED */ sa_family_t config_default_family =AF_INET ;
@@ -1410,13 +1405,15 @@ int nfs_nfs_proto_family(struct mount_options *options, } } -if (!nfs_verify_family (tmp_family)) - goto out_err; +#ifndef IPV6_SUPPORTED +if (!nfs_verify_family (tmp_family)) { + errno =EAFNOSUPPORT ; +return 0 ; + } +#endif + *family = tmp_family;return 1 ; -out_err: - errno =EAFNOSUPPORT ; -return 0 ; }/*
@@ -1566,8 +1563,10 @@ int nfs_mount_proto_family(struct mount_options *options, errno =EPROTONOSUPPORT ; goto out_err; } +#ifndef IPV6_SUPPORTED if (!nfs_verify_family (tmp_family)) goto out_err; +#endif *family = tmp_family;return 1 ; }
Signature · 1 line
1.7.0.1
Re: [PATCH 21/24] Removed warnings from network.c
Quoted 2 lines · Steve Dickson
network.c: In function 'nfs_verify_family':
network.c:1366: warning: unused parameter 'family'
I thought that the preferred coding style is to keep "#ifdef" out of mainline code by having alternate definitions of functions, as was done here.
A cleaner solution would be to leave the alternate definition of nfs_verify_family() and use
__attribute__((unused))
for the unused parameter.
Quoted 53 lines
Signed-off-by Steve Dickson<[email protected]>--- utils/mount/network.c | 19 +++++++++---------- 1 files changed, 9 insertions(+), 10 deletions(-)utils/mount/network.c +9
-10@@ -1362,11 +1362,6 @@ nfs_nfs_port(struct mount_options *options, unsigned long *port)#ifdef IPV6_SUPPORTED sa_family_t config_default_family =AF_UNSPEC ; -static int -nfs_verify_family (sa_family_t family) -{ -return 1 ; -}#else /* IPV6_SUPPORTED */ sa_family_t config_default_family =AF_INET ;@@ -1410,13 +1405,15 @@ int nfs_nfs_proto_family(struct mount_options *options, } } -if (!nfs_verify_family (tmp_family)) - goto out_err; +#ifndef IPV6_SUPPORTED +if (!nfs_verify_family (tmp_family)) { + errno =EAFNOSUPPORT ; +return 0 ; + } +#endif + *family = tmp_family;return 1 ; -out_err: - errno =EAFNOSUPPORT ; -return 0 ; }/* @@ -1566,8 +1563,10 @@ int nfs_mount_proto_family(struct mount_options *options, errno =EPROTONOSUPPORT ; goto out_err; } +#ifndef IPV6_SUPPORTED if (!nfs_verify_family (tmp_family)) goto out_err; +#endif *family = tmp_family;return 1 ; }
PATCH 22/24 Removed warnings from nfsmount.c
In reply to Steve Dicksonnfsmount.c: In function 'nfsmount':
nfsmount.c:513: warning: missing initializer nfsmount.c:513: warning: (near initialization for 'mnt_server.saddr') nfsmount.c:514: warning: missing initializer nfsmount.c:514: warning: (near initialization for 'nfs_server.saddr')
--- utils/mount/nfsmount.c | 8 ++++++-- 1 files changed, 6 insertions(+), 2 deletions(-)
utils/mount/nfsmount.c +6 -2
@@ -510,8 +510,12 @@ nfsmount(const char *spec, const char *node, int flags,int val;static int doonce =0 ; -clnt_addr_t mnt_server = { &mounthost, }; -clnt_addr_t nfs_server = { &hostname, }; +clnt_addr_t mnt_server = { + .hostname = &mounthost + }; +clnt_addr_t nfs_server = { + .hostname = &hostname + };struct sockaddr_in *nfs_saddr = &nfs_server.saddr;struct pmap *mnt_pmap = &mnt_server.pmap, *nfs_pmap = &nfs_server.pmap;
Signature · 1 line
1.7.0.1
PATCH 23/24 Removed warnings from nfs4mount.c
In reply to Steve Dicksonnfs4mount.c: In function 'fill_ipv4_sockaddr':
nfs4mount.c:149: warning: comparison between signed and unsigned integer expressions
--- utils/mount/nfs4mount.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
utils/mount/nfs4mount.c +1 -1
@@ -146,7 +146,7 @@ static int fill_ipv4_sockaddr(const char *hostname, struct sockaddr_in *addr) progname, hostname);return -1 ; } -if (hp->h_length >sizeof (struct in_addr )) { +if (hp->h_length > (int )sizeof (struct in_addr )) {nfs_error (_ ("%s: got bad hp->h_length" ), progname); hp->h_length =sizeof (struct in_addr ); }
Signature · 1 line
1.7.0.1
PATCH 24/24 Removed warnings from configfile.c
In reply to Steve Dicksonconfigfile.c:195: warning: 'inline' is not at beginning of declaration configfile.c:232: warning: 'inline' is not at beginning of declaration
--- utils/mount/configfile.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
utils/mount/configfile.c +2 -2
@@ -192,7 +192,7 @@ void free_all(void) } }static char *versions[] = {"v2" ,"v3" ,"v4" ,"vers" ,"nfsvers" ,NULL }; -int inline check_vers (char *mopt,char *field) +inline int check_vers (char *mopt,char *field) {int i, found=0 ;
@@ -229,7 +229,7 @@ extern sa_family_t config_default_family;* If so, set the appropriate global value which will * be used as the initial value in the server negation. */ -int inline default_value (char *mopt) +inline int default_value (char *mopt) {struct mount_options *options =NULL ;int dftlen =strlen ("default" );
Signature · 1 line
1.7.0.1
Re: [PATCH 24/24] Removed warnings from configfile.c
Hi Steve-
Quoted 13 lines · Steve Dickson
configfile.c:195: warning: 'inline' is not at beginning of declaration configfile.c:232: warning: 'inline' is not at beginning of declaration
Signed-off-by Steve Dickson<[email protected]>--- utils/mount/configfile.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)utils/mount/configfile.c +0
-0
utils/mount/configfile.c void free_all(void) · line 195
Quoted 5 lines
utils/mount/configfile.c +1
-1@@ -192,7 +192,7 @@ void free_all(void) } }static char *versions[] = {"v2" ,"v3" ,"v4" ,"vers" ,"nfsvers" ,NULL }; -int inline check_vers (char *mopt,char *field) +inline int check_vers (char *mopt,char *field)
I don't see the purpose of using inline here, nor of keeping this a global function. Is "inline" is addressing some Fortify warning?
If you're updating this, shouldn't it rather be:
static intcheck_vers(char *mopt, char *field)
?
utils/mount/configfile.c extern sa_family_t config_default_family; · line 235
Quoted 12 lines
utils/mount/configfile.c +1
-1{int i, found=0 ;@@ -229,7 +229,7 @@ extern sa_family_t config_default_family;* If so, set the appropriate global value which will * be used as the initial value in the server negation. */ -int inline default_value (char *mopt) +inline int default_value (char *mopt) {struct mount_options *options =NULL ;int dftlen =strlen ("default" );
And likewise...?