On 06/30/13 23:05, David Dillow wrote:
On Fri, 2013-06-28 at 14:53 +0200, Bart Van Assche wrote:+int srp_tmo_valid(int fast_io_fail_tmo, int dev_loss_tmo) +{ + return (fast_io_fail_tmo < 0 || dev_loss_tmo < 0 || + fast_io_fail_tmo < dev_loss_tmo) && + fast_io_fail_tmo < LONG_MAX / HZ && + dev_loss_tmo < LONG_MAX / HZ ? 0 : -EINVAL; +}They should also be capped by SCSI_DEVICE_BLOCK_MAX_TIMEOUT instead of LONG_MAX / HZ, I think.
The fast_io_fail_tmo should indeed be capped by that value. However, I'm not sure about dev_loss_tmo. I think there are several use cases (e.g. initiator-side mirroring) where it's useful to set dev_loss_tmo to a larger value than ten minutes.
+static ssize_t store_srp_rport_fast_io_fail_tmo(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + struct srp_rport *rport = transport_class_to_srp_rport(dev); + char ch[16], *p; + int res; + int fast_io_fail_tmo; + + sprintf(ch, "%.*s", min_t(int, sizeof(ch) - 1, count), buf); + p = strchr(ch, '\n'); + if (p) + *p = '\0';Again, no need for the sprintf if you don't modify the buffer? Instead of using strchr() to make the strcmp() work with newlines, just do if (!strcmp(buf, "off") || !strcmp(buf, "off\n")) { fast_io_fail_tmo = 1; } else { res = kstrtoint(buf, 0, &fast_io_fail_tmo); ... instead? Same comment applies for store_srp_rport_dev_loss_tmo().
OK, will remove the temporary char arrays. Bart. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
