Re: [PATCH] /dev/time for Linux, inspired by Plan 9

[AmÃrico Wang]

On Tue, Mar 03, 2009 at 09:06:46PM -0600, Christopher Brannon wrote:

<snip>

>@@ -0,0 +1,112 @@
>+#include <linux/init.h>
>+#include <linux/module.h>
>+#include <linux/fs.h>
>+#include <linux/miscdevice.h>
>+#include <linux/time.h>
>+#include <linux/jiffies.h>
>+#include <linux/uaccess.h>
>+
>+MODULE_AUTHOR("Christopher Brannon <cmbrannon79@xxxxxxxxx>");
>+MODULE_LICENSE("GPL");
>+
>+const long long NS_PER_SEC = 1000000000;
>+const long long NS_PER_USEC = 1000;
>+const size_t time_bufsize = 256;

I notice that you use this "const" as an array size below, so
you are using VLA, yes, C99 supports it but this is in the kernel,
try to avoid this by using marco:

#define BUFSIZE 256

And yes, C's const sucks. :)


>+
>+static ssize_t time_read(struct file *, char __user *, size_t, loff_t *);
>+static ssize_t time_write(struct file *, const char __user *, size_t, loff_t *);
>+
>+static const struct file_operations time_fops = {
>+	.owner = THIS_MODULE,
>+	.read = time_read,
>+	.write = time_write,
>+};
>+
>+static struct miscdevice timedev = {
>+	.minor = MISC_DYNAMIC_MINOR,
>+	.name = "time",
>+	.fops = &time_fops
>+};
>+
>+static int time2text(char *buffer, size_t bufsize)
>+{
>+	int count = 0;
>+	struct timeval tv;
>+	long long nanos;
>+
>+/* jiffies isn't 0 at boot time; its value is INITIAL_JIFFIES. */
>+	u64 ticks = get_jiffies_64() - INITIAL_JIFFIES;
>+
>+	do_gettimeofday(&tv);
>+	nanos = tv.tv_sec * NS_PER_SEC + tv.tv_usec * NS_PER_USEC;
>+	count =
>+	    scnprintf(buffer, bufsize, "%ld %lld %llu %d\n", tv.tv_sec, nanos,
>+		      ticks, HZ);
>+	return count;
>+}
>+
>+static int text2time(char *buffer)
>+{
>+	struct timespec tv;
>+	int result = strict_strtol(buffer, 10, &tv.tv_sec);
>+	if ((result == 0) && (tv.tv_sec > 0)) {
>+		tv.tv_nsec = 0;
>+		do_settimeofday(&tv);
>+	} else
>+		result = -EINVAL;	/* only positive longs are valid. */
>+	return result;
>+}
>+
>+static ssize_t
>+time_read(struct file *f, char __user *buffer, size_t count, loff_t * offset)
>+{
>+	int result = 0;
>+	if (*offset != 0)
>+		result = 0;
>+	else {
>+		char tmpbuf[time_bufsize];
>+		int timetextlen = time2text(tmpbuf, time_bufsize);
>+		unsigned long readcount = min(count, (size_t) timetextlen);
>+		if (timetextlen <= 0)
>+			return -EAGAIN;
>+		if (!copy_to_user(buffer, tmpbuf, readcount)) {
>+			*offset += readcount;
>+			result = readcount;
>+		} else
>+			result = -EFAULT;
>+	}
>+	return result;
>+}
>+
>+static ssize_t
>+time_write(struct file *f, const char __user * buffer, size_t count,
>+	   loff_t *offset)
>+{
>+	unsigned int result = 0;
>+	char tmpbuf[time_bufsize];
>+
>+	if (*offset != 0)
>+		return -EINVAL;
>+	if (count > ((size_t) time_bufsize - 1))
>+		return -EINVAL;	/* Likely trying to feed bogus data anyway. */
>+	result = copy_from_user(tmpbuf, buffer, count);
>+	if (result)
>+		return -EFAULT;
>+	tmpbuf[count] = '\0';
>+	if (text2time(tmpbuf))
>+		return -EINVAL;
>+	return count;
>+}
>+
>+static int __init time_init(void)
>+{
>+	return misc_register(&timedev);
>+}
>+
>+static void __exit time_exit(void)
>+{
>+	misc_deregister(&timedev);
>+}
>+
>+module_init(time_init);
>+module_exit(time_exit);
>-- 
>1.6.1.3
>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>the body of a message to majordomo@xxxxxxxxxxxxxxx
>More majordomo info at  http://vger.kernel.org/majordomo-info.html
>Please read the FAQ at  http://www.tux.org/lkml/

-- 
Do what you love, f**k the rest! F**k the regulations!
 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/