At 03:32 05/12/01, Nathan Scott wrote:
Here is the revised interface. I believe it takes into account
the issues raised so far - further suggestions are also welcome,
of course.
Hi,
Looks good to me. Just one tiny point: you seem to like setting error=xyz;
a lot which is completely unnecessary some times. Any particular reason?
Here is an example of what I mean:
+static long
+setxattr(struct dentry *d, char *name, void *value, size_t size, int flags)
+{
+ int error;
+ void *kvalue;
+ char kname[XATTR_NAME_MAX + 1];
+
+ error = -EINVAL;
+ if (flags & ~(XATTR_CREATE|XATTR_REPLACE))
+ return error;
Why not:
+ if (flags & ~(XATTR_CREATE|XATTR_REPLACE))
+ return -EINVAL;
+
+ error = -EFAULT;
+ if (copy_from_user(kname, name, XATTR_NAME_MAX))
+ return error;
+ if (copy_from_user(kname, name, XATTR_NAME_MAX))
+ return -EFAULT;
+ kname[XATTR_NAME_MAX] = '\0';
+
+ kvalue = xattr_alloc(size, XATTR_SIZE_MAX);
+ if (IS_ERR(kvalue))
+ return PTR_ERR(kvalue);
+
+ error = -EFAULT;
+ if (size > 0 && copy_from_user(kvalue, value, size)) {
+ xattr_free(kvalue, size);
+ return error;
+ }
+ if (size > 0 && copy_from_user(kvalue, value, size)) {
+ xattr_free(kvalue, size);
+ return -EFAULT;
+ }
Shorter, faster in the common non-error path, and looks nicer, although the
latter is probably a matter of personal preference. (-;
Best regards,
Anton
--
"I've not lost my mind. It's backed up on tape somewhere." - Unknown
--
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Linux NTFS Maintainer / WWW: http://linux-ntfs.sf.net/
ICQ: 8561279 / WWW: http://www-stu.christs.cam.ac.uk/~aia21/
|