2006-11-09 03:44:53

by NeilBrown

[permalink] [raw]
Subject: Re: [PATCH 05/12] SUNRPC: Add a function to format the address in an svc_rqst for printing

On Wednesday November 1, [email protected] wrote:
> long timeout = MAX_SCHEDULE_TIMEOUT;
> + char buf[RPC_MAX_ADDRBUFLEN];
>
> if (signalled()) {
> flush_signals(current);
> @@ -175,11 +176,10 @@ lockd(struct svc_rqst *rqstp)
> break;
> }
>
> - dprintk("lockd: request from %08x\n",
> - (unsigned)ntohl(rqstp->rq_addr.sin_addr.s_addr));
> + svc_print_addr(rqstp, buf, sizeof(buf));
> + dprintk("lockd: request from %s\n", buf);
>
....
> +/**
> + * svc_print_addr - Format rq_addr field for printing
> + * @rqstp: svc_rqst struct containing address to print
> + * @buf: target buffer for formatted address
> + * @len: length of target buffer
> + *
> + */
> +void svc_print_addr(struct svc_rqst *rqstp, char *buf, size_t len)
> +{
> + __svc_print_addr((struct sockaddr *) &rqstp->rq_addr, buf, len);
> +
> +}

Can we have svc_print_addr take a
char buf[RPC_MAX_ADDRBUFLEN]
rather than a char* and a len? The buffer is always the same size, so
best to hard code it..

Also can svc_print_addr return a 'char *' being 'buf'.
Then we can

> + dprintk("lockd: request from %s\n",
> + svc_print_addr(rqstp, buf));

This makes it less verbose and similar to bdevname().

Thanks,
NeilBrown

-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs


2006-11-09 10:16:45

by Olaf Kirch

[permalink] [raw]
Subject: Re: [PATCH 05/12] SUNRPC: Add a function to format the address in an svc_rqst for printing

On Thu, Nov 09, 2006 at 02:44:50PM +1100, Neil Brown wrote:
> > +void svc_print_addr(struct svc_rqst *rqstp, char *buf, size_t len)
> > +{
> > + __svc_print_addr((struct sockaddr *) &rqstp->rq_addr, buf, len);
> > +
> > +}
>
> Can we have svc_print_addr take a
> char buf[RPC_MAX_ADDRBUFLEN]
> rather than a char* and a len? The buffer is always the same size, so
> best to hard code it..

The compiler will still let you pass a buf[6] without complaining.
Assumptions that buffer arguments are "big enough" make for great
security bugs two years down the road because people forget.
There's a reason why getwd and gets have been deprecated for 18 years :)

If you want to make it easy, why not have

#define RPC_SVC_ADDR(rqstp, buf) \
(svc_print_addr(rqstp, buf, sizeof(buf)))

> Also can svc_print_addr return a 'char *' being 'buf'.

That'd be nice.

Olaf
--
Walks like a duck. Quacks like a duck. Must be a chicken.

-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs