Thanks Meem. Please see in-line. > > > > http://cr.opensolaris.org/~girishmg/libdlpi/ > > Thanks for picking this up. A few comments: > > * The new logic will plow on to line 1493 and beyond if it > receives a malformed DL_NOTIFY_IND. It should throw it away > and continue like the old code did at line 1483. > See below. > * Similarly, I'm confused why we no longer `continue' after > processing the DL_NOTIFY_IND. That is, why was 1487 removed? > Two Reasons:
Reason 1: Because we will skip the entire *msec* update at the bottom of the 'for' loop. Definitely fair amount of time would have passed in sending the notification UP to the consumers, right? Well we could argue we shouldn't count the time spent in sending out the notifications and deleting notification nodes (if any) in i_dlpi_notifyind_process(). I wasn't sure about the behavior here and wanted to catch it in code-review. Also the three 'if' conditions after L1493, i.e. @L1499, @L1513, at L1518 wouldn't be true for DL_NOTIFY_IND case. So we would go past it to execute the 'msec' update and return back to poll() at the beginning of the 'for' loop. Reason 2: (no more holds good) I was of the opinion that getmsg() would also fill in the data buffer because I thought there could also be M_DATA block for DL_NOTIFY_IND. But I realized that it isn't true and my understanding was incorrect. > * Please remove all the additional ()'s you've added. It just > clutters up the code. Likewise for the needless `int' cast > on line 1486; these types are compatible without the cast. > Done. > * Rather than pepper LINTED's in, please just cast ctl.buf to > `void *' first -- e.g.: > > (*(t_uscalar_t *)(void *)ctl.buf == DL_NOTIFY_IND > > This accomplishes the same thing (clues lint in that the cast is > safe) without having to annotate the code. Done > > > * In the comment at 1395-1398, should it be "; see > dlpi_enabnotify(3DLPI)"? Also, the second sentence doesn't > seem important here. > Done thanks ~Girish
