This looks like a huge win in terms of code cleanup. Comments below.
Why are min and max args 'int's instead of 'size_t's or 'unsigned'?
In ofproto/trace, you have a case which expects argc == 6, but you
only allow a maximum of 4 (+ 1 = 5) arguments when you register. It
looks like you are leaving off the priority field in the usage of the
register call.
The argument parsing of ofproto/trace is pretty confusing in general.
I wonder if it would be worth unit testing all of the possible code
paths beyond what we're already doing for the ofproto-dpif unit tests.
> @@ -803,11 +803,8 @@ netdev_linux_recv(struct netdev *netdev_, void *data,
> size_t size)
>
> for (;;) {
> ssize_t retval = recv(netdev->fd, data, size, MSG_TRUNC);
> - if (retval > size) {
> - /* Received packet was longer than supplied buffer. */
> - return -EMSGSIZE;
> - } else if (retval >= 0) {
> - return retval;
> + if (retval >= 0) {
> + return retval <= size ? retval : -EMSGSIZE;
This looks like a rebasing error to me. Was it intended for this patch?
> + for (i = 0; i < n_stress_options; i++) {
> + struct stress_option *option = stress_options[i];
> + if (!strcmp(option_name, option->name)) {
> + unsigned int period = strtoul(option_val, NULL, 0);
> + bool random = !strcmp(argv[3], "random");
argv[3] could be NULL here.
> + unixctl_command_register(
> + "vlog/set", "{module[:facility[:level]] | PATTERN:facility:pattern}",
I might be inclined to add a ... to the end of the usage argument.
However, I don't know what standard practice for this sort of thing
is.
Ethan
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
