On Nov 29, 2007, at 7:53 PM, Trond Myklebust wrote:
> On Thu, 2007-11-29 at 14:40 -0500, Chuck Lever wrote:
>> Add support to nfs_verify_server_address for recognizing AF_INET6
>> addresses.
>>
>> Signed-off-by: Chuck Lever <[email protected]>
>> ---
>>
>> fs/nfs/super.c | 27 ++++++++++++++++-----------
>> 1 files changed, 16 insertions(+), 11 deletions(-)
>>
>> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
>> index e5646a9..ed0b5f1 100644
>> --- a/fs/nfs/super.c
>> +++ b/fs/nfs/super.c
>> @@ -576,16 +576,21 @@ static void nfs_umount_begin(struct vfsmount
>> *vfsmnt, int flags)
>> }
>>
>> /*
>> - * Sanity-check a server address provided by the mount command
>> + * Sanity-check a server address provided by the mount command.
>> + *
>> + * Address family must be initialized, and address must not be
>> + * the ANY address for that family.
>> */
>> -static int nfs_verify_server_address(struct sockaddr *addr)
>> +static int nfs_verify_server_address(struct sockaddr_storage *ssp)
>
> Hmm... This looks wrong. We should never be using a type 'pointer to
> sockaddr_storage'. Particularly not when all the callers have to cast.
> Please convert to a generic 'struct sockaddr *'.
>
> Ditto in the other cases (nfs4_default_port(),
> nfs_parse_server_address() etc.).
I've found instances of using "struct sockaddr_storage *" in other
parts of the kernel. Take a look at fs/dlm/* for example.
I assume by now you've seen the whole patch series, and see that
indeed, the type casts (including the "void *"s I added, which are
temporary) aren't needed by the time we're finished, and are removed
by the last patch.
Using "struct sockaddr_storage *" for these internal helpers means we
don't need any type casts when passing the nfs_server.address field.
Using "struct sockaddr *" means we have to cast the
nfs_server.address field every time one of these is called. I use
"struct sockaddr_storage *" only for internal (ie static) functions,
not ever as part of an exported interface.
Take a look at fs/nfs/super.c after the last patch is applied and you
will see my point.
> BTW: did I miss [Patch 11/29]? I can't find it in my inbox...
Looks like I didn't get 11 via the mailing list either, so it must
have been dropped somewhere. I'll resend it.
> Trond
>
>> {
>> - switch (addr->sa_family) {
>> + switch (ssp->ss_family) {
>> case AF_INET: {
>> - struct sockaddr_in *sa = (struct sockaddr_in *) addr;
>> - if (sa->sin_addr.s_addr != INADDR_ANY)
>> - return 1;
>> - break;
>> + struct sockaddr_in *sa = (struct sockaddr_in *)ssp;
>> + return sa->sin_addr.s_addr != INADDR_ANY;
>> + }
>> + case AF_INET6: {
>> + struct in6_addr *sa = &((struct sockaddr_in6 *)ssp)->sin6_addr;
>> + return !ipv6_addr_any(sa);
>> }
>> }
>>
>> @@ -1084,7 +1089,7 @@ static int nfs_validate_mount_data(void
>> *options,
>> memset(mntfh->data + mntfh->size, 0,
>> sizeof(mntfh->data) - mntfh->size);
>>
>> - if (!nfs_verify_server_address((struct sockaddr *) &data->addr))
>> + if (!nfs_verify_server_address((void *)&data->addr))
>> goto out_no_address;
>>
>> /*
>> @@ -1118,7 +1123,7 @@ static int nfs_validate_mount_data(void
>> *options,
>> if (nfs_parse_mount_options((char *)options, args) == 0)
>> return -EINVAL;
>>
>> - if (!nfs_verify_server_address((struct sockaddr *)
>> + if (!nfs_verify_server_address((void *)
>> &args->nfs_server.address))
>> goto out_no_address;
>>
>> @@ -1577,7 +1582,7 @@ static int nfs4_validate_mount_data(void
>> *options,
>> return -EFAULT;
>> if (args->nfs_server.address.sin_port == 0)
>> args->nfs_server.address.sin_port = htons(NFS_PORT);
>> - if (!nfs_verify_server_address((struct sockaddr *)
>> + if (!nfs_verify_server_address((void *)
>> &args->nfs_server.address))
>> goto out_no_address;
>>
>> @@ -1636,7 +1641,7 @@ static int nfs4_validate_mount_data(void
>> *options,
>>
>> if (args->nfs_server.address.sin_port == 0)
>> args->nfs_server.address.sin_port = htons(NFS_PORT);
>> - if (!nfs_verify_server_address((struct sockaddr *)
>> + if (!nfs_verify_server_address((void *)
>> &args->nfs_server.address))
>> return -EINVAL;
>>
>>
>
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
-------------------------------------------------------------------------
SF.Net email is sponsored by: The Future of Linux Business White Paper
from Novell. From the desktop to the data center, Linux is going
mainstream. Let it simplify your IT future.
http://altfarm.mediaplex.com/ad/ck/8857-50307-18918-4
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
_______________________________________________
Please note that [email protected] is being discontinued.
Please subscribe to [email protected] instead.
http://vger.kernel.org/vger-lists.html#linux-nfs