Some further code review comments on another file.

Please check all files for "#if 0"-"#endif"...

usr/src/uts/common/fs/sockfs/socksyscalls.c 1579-1628
Type: T
Priority: 1
Here have you have a collection of if() statements that
seem suggest that all of the configuration is not being
driven by soconfig(1m). The presence of this code makes
me believe that something else is not right with either
the design or implementation of this project. The reason
for this is that it appears you are hard coding a translation
table into the kernel that mirrors what is normally found
in /etc/sock2path for "sockmods". What limitations does
this put on the types of sockets that can be cofigured
through volo?


usr/src/uts/common/fs/sockfs/socksyscalls.c 623-631
Type: T
Priority: 2
Comment: It is acceptable (and not uncommon) to call accept()
like this - accept(fd, NULL, 0) - when you're not interested in
receiving remote address information via accept. In the old code
this only meant a call to copyout() for 0 bytes, however now we
are always doing a getpeername.


usr/src/uts/common/fs/sockfs/socksyscalls.c 586-602
Type: T
Priority: 3
Comment: Although not related to this project's changes, if you
are looking to speed up accept(3socket), you can probably delete
some code here.What this code appears to do is ensure that the
memory pointed to by name/namelen can hold an address before
we call copyout() later. Given that we still need to check the
return of copyout(), this seems to be a needless duplication of
effort. It could, however, be reduced to saying that if either
name or namelen are NULL/0 then both should be.

usr/src/uts/common/fs/sockfs/socksyscalls.c 1328-1352
Type: T
Priority: 2
Comment: You're using kmem_alloc to allocate a scratch space
for an internal call. Whilst the amount of memory you are allocating
appears variable (so_max_addr_len), it's actually fixed at
sizeof(sockaddr_stoage). I suspect that so_max_addr_len is a bit
of a misnomer unless there is something that uses a larger address
size than this structure. Anyway, it would appear that you should
be able to do away with the kmem_alloc() call completely and just
use a local variable that is a sockaddr_storage. This will save you
some kmem_alloc's and kmem_free's (better performance.)
The same applies to getsockname().


_______________________________________________
networking-discuss mailing list
[email protected]

Reply via email to