On Thu, Apr 15, 2010 at 12:50 PM, Justin Pettit <span dir="ltr"><<a href="mailto:jpettit@nicira.com">jpettit@nicira.com</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div class="im">On Apr 15, 2010, at 9:34 AM, Ben Pfaff wrote:<br>
<br>
> In create_patch(), || is treated as if it yields the first nonzero<br>
> value, but actually in C it always yields 0 or 1, so this will squash<br>
> all errno values to 1 (EPERM as it happens on Linux).<br>
<br>
</div>I did not know that. (I originally wrote that as "Very interesting", but I'm not sure I actually feel that way. ;-) But seriously, thank you for teaching me that.) Fixed.<br>
<div class="im"><br>
> In setup_patch(), if you do care about unknown arguments, you could<br>
> check for shash_count(args) > 1.<br>
<br>
</div>Good point. I added that.<br>
<div class="im"><br>
> In setup_patch(), I think that IFNAMSIZ - 1 would be a more accurate<br>
> maximum device name length, because IFNAMSIZE has to include the<br>
> trailing null byte.<br>
<br>
<br>
</div>I don't know what I was thinking there.<br>
<br>
Thanks for the review. I'll wait to hear back from Jesse before I push.</blockquote><div><br></div><div> This looks a lot better to me. Thanks for going back over it.</div></div>