deepti dhokte wrote:
Hi
Anders,
As told, here are some review comments.
1)
in.ndpd/main.c :
L1514 -
L1689 -
Add comment block above new function definition.
Sure.
you might consider doing a validity check for passed arguments, if
applicable.
The argument passed in is a socket descriptor. If we can not open the
socket the program will exit. So there is not need to check here.
L1531-
Do buffer overflow check for strncpy.
We avoided that because in this case there can not ever be an over flow
since the unix socket name
define NDPD_SNMP_SOCKET "/var/run/in.ndpd_mib"
is constant and will fit the buffer.
L1696-1703 -
len is of type size_t, later it is being compared with sizeof (int).
shouldn't len be declared of type int ?
This is a typo. It should be defined as ssize_t. Will it.
2)
in.ndpd/tables.h :
just a thought -
Shouldn't this entire file be renamed as ndpd.h,
tables.h as a file name of in.ndpd daemon, seems inconsistent to me.
This is a local file and contains information about the tables the
daemon keeps. I think the name is just fine.
3)
in.routed/defs.h:
L167-
add one liner comment above extern declaration for struct dr.
You could choose self-explanatory names for variables.
We will add a one liner. However this is not a variable that we defined
we would like to leave the name unchanged.
4)
in.routed/input.c:
L1063 - change it to "Honor only those requests ... "
OK.
L1096-
Don't we need RIPCMD_DEFROUTERS_RESP implemented as well,
we declare the ripcmd (DEFROUTERS_RESP at L-58 in in.routed/trace.c)?
RIPCMD_DEFROUTERS_REQ/RIPCMD_DEFROUTERS_RESP are Solaris specific
commands. in.routed can only receive a request.
5)
in.routed/rdisc.c
L90
L91
Same, you can add comment or you may choose to be descriptive with
variable names.
Again these are not variables that we introduced.
5)
protocols/routed.h
L157:
Rename defr_t to def_rtr_t for better understanding
OK.
(b)
Do Null pointer dereference checks before updating mibs for ill, i.e.
ill->ill_ip_mib.
As Anders has pointed out doing unnecessary check is not necessary.
Adding an assert is good enough.
<...>
Anders has addressed other comments. I will add if necessary.
Rao.
_______________________________________________
networking-discuss mailing list
[email protected]
