* Re: [PATCH 1/7] SUNRPC: Address a few compiler warnings in the RPC client
[not found] ` <1188238002.6701.99.camel@heimdal.trondhjem.org>
@ 2007-08-27 19:15 ` Chuck Lever
2007-08-27 21:59 ` Trond Myklebust
0 siblings, 1 reply; 2+ messages in thread
From: Chuck Lever @ 2007-08-27 19:15 UTC (permalink / raw)
To: Trond Myklebust; +Cc: nfs
[-- Attachment #1: Type: text/plain, Size: 1905 bytes --]
>> diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
>> index c796e2f..0ad3042 100644
>> --- a/net/sunrpc/clnt.c
>> +++ b/net/sunrpc/clnt.c
>> @@ -127,7 +127,7 @@ static struct rpc_clnt * rpc_new_client(struct rpc_xprt *xprt, char *servname, s
>> struct rpc_clnt *clnt = NULL;
>> struct rpc_auth *auth;
>> int err;
>> - int len;
>> + size_t len;
>
> What if 'len' overflows when we do 'strlen() + 1'?
strlen() returns size_t, which is unsigned. Thus the overflow would be
zero. Seems like that's a better outcome than what would happen today.
>> dprintk("RPC: creating %s client for %s (xprt %p)\n",
>> program->name, servname, xprt);
>> @@ -436,7 +436,7 @@ rpc_release_client(struct rpc_clnt *clnt)
>> */
>> struct rpc_clnt *rpc_bind_new_program(struct rpc_clnt *old,
>> struct rpc_program *program,
>> - int vers)
>> + unsigned int vers)
>> {
>> struct rpc_clnt *clnt;
>> struct rpc_version *version;
>> @@ -1508,6 +1508,7 @@ void rpc_show_tasks(void)
>> {
>> struct rpc_clnt *clnt;
>> struct rpc_task *t;
>> + int proc = -1;
>>
>> spin_lock(&rpc_client_lock);
>> if (list_empty(&all_clients))
>> @@ -1524,9 +1525,11 @@ void rpc_show_tasks(void)
>> if (RPC_IS_QUEUED(t))
>> rpc_waitq = rpc_qname(t->u.tk_wait.rpc_waitq);
>>
>> + if (t->tk_msg.rpc_proc)
>> + proc = t->tk_msg.rpc_proc->p_proc;
>> +
>> printk("%5u %04d %04x %6d %8p %6d %8p %8ld %8s %8p %8p\n",
>> - t->tk_pid,
>> - (t->tk_msg.rpc_proc ? t->tk_msg.rpc_proc->p_proc : -1),
>> + t->tk_pid, proc,
>> t->tk_flags, t->tk_status,
>> t->tk_client,
>> (t->tk_client ? t->tk_client->cl_prog : 0),
>
> NACK. Look again at your change to rpc_show_tasks. What you are doing
> with your variable 'proc' is _not_ equivalent to the existing code: you
> need to reset proc to '-1' each time you go through the loop.
Noted and fixed.
[-- Attachment #2: chuck.lever.vcf --]
[-- Type: text/x-vcard, Size: 302 bytes --]
begin:vcard
fn:Chuck Lever
n:Lever;Chuck
org:Oracle Corporation;Corporate Architecture: Linux Projects Group
adr:;;1015 Granger Avenue;Ann Arbor;MI;48104;USA
title:Principal Member of Staff
tel;work:+1 248 614 5091
x-mozilla-html:FALSE
url:http://oss.oracle.com/~cel
version:2.1
end:vcard
[-- Attachment #3: Type: text/plain, Size: 315 bytes --]
-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
[-- Attachment #4: Type: text/plain, Size: 140 bytes --]
_______________________________________________
NFS maillist - NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs
^ permalink raw reply [flat|nested] 2+ messages in thread