Re: [PATCH 02/10] Add yaffs2 file system: attrib and xattrib handling
From: Ryan Mallon <hidden>
Date: 2011-02-09 22:33:42
Also in:
lkml
On 02/09/2011 04:26 PM, Charles Manning wrote:
Signed-off-by: Charles Manning <redacted>
Hi Charles, Some comments below, ~Ryan
quoted hunk ↗ jump to hunk
--- fs/yaffs2/yaffs_attribs.c | 124 ++++++++++++++++++++++++++++ fs/yaffs2/yaffs_attribs.h | 28 ++++++ fs/yaffs2/yaffs_nameval.c | 201 +++++++++++++++++++++++++++++++++++++++++++++ fs/yaffs2/yaffs_nameval.h | 28 ++++++ 4 files changed, 381 insertions(+), 0 deletions(-) create mode 100644 fs/yaffs2/yaffs_attribs.c create mode 100644 fs/yaffs2/yaffs_attribs.h create mode 100644 fs/yaffs2/yaffs_nameval.c create mode 100644 fs/yaffs2/yaffs_nameval.hdiff --git a/fs/yaffs2/yaffs_attribs.c b/fs/yaffs2/yaffs_attribs.c new file mode 100644 index 0000000..fe914e5 --- /dev/null +++ b/fs/yaffs2/yaffs_attribs.c@@ -0,0 +1,124 @@ +/* + * YAFFS: Yet Another Flash File System. A NAND-flash specific file system. + * + * Copyright (C) 2002-2011 Aleph One Ltd. + * for Toby Churchill Ltd and Brightstar Engineering + * + * Created by Charles Manning <charles@aleph1.co.uk> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include "yaffs_guts.h" +#include "yaffs_attribs.h" + +void yaffs_load_attribs(struct yaffs_obj *obj, struct yaffs_obj_hdr *oh) +{ + obj->yst_uid = oh->yst_uid; + obj->yst_gid = oh->yst_gid; + obj->yst_atime = oh->yst_atime; + obj->yst_mtime = oh->yst_mtime; + obj->yst_ctime = oh->yst_ctime; + obj->yst_rdev = oh->yst_rdev; +} + +void yaffs_load_attribs_oh(struct yaffs_obj_hdr *oh, struct yaffs_obj *obj) +{ + oh->yst_uid = obj->yst_uid; + oh->yst_gid = obj->yst_gid; + oh->yst_atime = obj->yst_atime; + oh->yst_mtime = obj->yst_mtime; + oh->yst_ctime = obj->yst_ctime; + oh->yst_rdev = obj->yst_rdev; + +} + +void yaffs_load_current_time(struct yaffs_obj *obj, int do_a, int do_c) +{ + obj->yst_mtime = Y_CURRENT_TIME; + if (do_a) + obj->yst_atime = obj->yst_mtime; + if (do_c) + obj->yst_ctime = obj->yst_mtime; +} + +void yaffs_attribs_init(struct yaffs_obj *obj, u32 gid, u32 uid, u32 rdev) +{ + yaffs_load_current_time(obj, 1, 1); + obj->yst_rdev = rdev; + obj->yst_uid = uid; + obj->yst_gid = gid; +} + +loff_t yaffs_get_file_size(struct yaffs_obj *obj) +{ + YCHAR *alias = NULL; + obj = yaffs_get_equivalent_obj(obj); + + switch (obj->variant_type) { + case YAFFS_OBJECT_TYPE_FILE: + return obj->variant.file_variant.file_size; + case YAFFS_OBJECT_TYPE_SYMLINK: + alias = obj->variant.symlink_variant.alias; + if (!alias) + return 0; + return strnlen(alias, YAFFS_MAX_ALIAS_LENGTH); + default: + return 0; + } +} + +int yaffs_set_attribs(struct yaffs_obj *obj, struct iattr *attr) +{ + unsigned int valid = attr->ia_valid; + + if (valid & ATTR_MODE) + obj->yst_mode = attr->ia_mode; + if (valid & ATTR_UID) + obj->yst_uid = attr->ia_uid; + if (valid & ATTR_GID) + obj->yst_gid = attr->ia_gid; + + if (valid & ATTR_ATIME) + obj->yst_atime = Y_TIME_CONVERT(attr->ia_atime); + if (valid & ATTR_CTIME) + obj->yst_ctime = Y_TIME_CONVERT(attr->ia_ctime); + if (valid & ATTR_MTIME) + obj->yst_mtime = Y_TIME_CONVERT(attr->ia_mtime); + + if (valid & ATTR_SIZE) + yaffs_resize_file(obj, attr->ia_size); + + yaffs_update_oh(obj, NULL, 1, 0, 0, NULL); + + return YAFFS_OK; + +} + +int yaffs_get_attribs(struct yaffs_obj *obj, struct iattr *attr) +{ + unsigned int valid = 0; + + attr->ia_mode = obj->yst_mode; + valid |= ATTR_MODE; + attr->ia_uid = obj->yst_uid; + valid |= ATTR_UID; + attr->ia_gid = obj->yst_gid; + valid |= ATTR_GID; + + Y_TIME_CONVERT(attr->ia_atime) = obj->yst_atime; + valid |= ATTR_ATIME; + Y_TIME_CONVERT(attr->ia_ctime) = obj->yst_ctime; + valid |= ATTR_CTIME; + Y_TIME_CONVERT(attr->ia_mtime) = obj->yst_mtime; + valid |= ATTR_MTIME; + + attr->ia_size = yaffs_get_file_size(obj); + valid |= ATTR_SIZE; + + attr->ia_valid = valid; + + return YAFFS_OK; +}
Is there are reason this cannot be written as:
int yaffs_get_attribs(struct yaffs_obj *obj, struct iattr *attr)
{
attr->ia_mode = obj->yst_mode;
attr->ia_uid = obj->yst_uid;
attr->ia_gid = obj->yst_gid;
Y_TIME_CONVERT(attr->ia_atime) = obj->yst_atime;
Y_TIME_CONVERT(attr->ia_ctime) = obj->yst_ctime;
Y_TIME_CONVERT(attr->ia_mtime) = obj->yst_mtime;
attr->ia_size = yaffs_get_file_size(obj);
attr->ia_valid = (ATTR_MODE | ATTR_UID | ATTR_GID |
ATTR_ATIME | ATTR_CTIME | ATTR_MTIME |
ATTR_SIZE);
return YAFFS_OK;
}
Which is a bit more readable?
quoted hunk ↗ jump to hunk
diff --git a/fs/yaffs2/yaffs_attribs.h b/fs/yaffs2/yaffs_attribs.h new file mode 100644 index 0000000..5b21b08 --- /dev/null +++ b/fs/yaffs2/yaffs_attribs.h@@ -0,0 +1,28 @@ +/* + * YAFFS: Yet another Flash File System . A NAND-flash specific file system. + * + * Copyright (C) 2002-2011 Aleph One Ltd. + * for Toby Churchill Ltd and Brightstar Engineering + * + * Created by Charles Manning <charles@aleph1.co.uk> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License version 2.1 as + * published by the Free Software Foundation. + * + * Note: Only YAFFS headers are LGPL, YAFFS C code is covered by GPL. + */ + +#ifndef __YAFFS_ATTRIBS_H__ +#define __YAFFS_ATTRIBS_H__ + +#include "yaffs_guts.h" + +void yaffs_load_attribs(struct yaffs_obj *obj, struct yaffs_obj_hdr *oh); +void yaffs_load_attribs_oh(struct yaffs_obj_hdr *oh, struct yaffs_obj *obj); +void yaffs_attribs_init(struct yaffs_obj *obj, u32 gid, u32 uid, u32 rdev); +void yaffs_load_current_time(struct yaffs_obj *obj, int do_a, int do_c); +int yaffs_set_attribs(struct yaffs_obj *obj, struct iattr *attr); +int yaffs_get_attribs(struct yaffs_obj *obj, struct iattr *attr); + +#endifdiff --git a/fs/yaffs2/yaffs_nameval.c b/fs/yaffs2/yaffs_nameval.c new file mode 100644 index 0000000..e75411b --- /dev/null +++ b/fs/yaffs2/yaffs_nameval.c@@ -0,0 +1,201 @@ +/* + * YAFFS: Yet Another Flash File System. A NAND-flash specific file system. + * + * Copyright (C) 2002-2011 Aleph One Ltd. + * for Toby Churchill Ltd and Brightstar Engineering + * + * Created by Charles Manning <charles@aleph1.co.uk> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +/* + * This simple implementation of a name-value store assumes a small number of +* values and fits into a small finite buffer. + * + * Each attribute is stored as a record: + * sizeof(int) bytes record size. + * strnlen+1 bytes name null terminated. + * nbytes value. + * ---------- + * total size stored in record size
This code feels like it should have helper functions to minimise the amount of pointer arithmetic and memcpy magic.
+ *
+ * This code has not been tested with unicode yet.
+ */
+
+#include "yaffs_nameval.h"
+
+#include "yportenv.h"
+
+static int nval_find(const char *xb, int xb_size, const YCHAR *name,
+ int *exist_size)
+{
+ int pos = 0;
+ int size;
+
+ memcpy(&size, xb, sizeof(int));
+ while (size > 0 && (size < xb_size) && (pos + size < xb_size)) {You don't need thr parens around the expressions here.
+ if (!strncmp((YCHAR *) (xb + pos + sizeof(int)), name, size)) {
Make a helper function:
static YCHAR *nval_name(const char *xb, int pos)
{
return (YCHAR *)(xb + pos + sizeof(int));
}
Then:
if (!strncmp(nval_name(xb, pos), name, size)) {
Much easier to read.
+ if (exist_size) + *exist_size = size; + return pos; + } + pos += size; + if (pos < xb_size - sizeof(int)) + memcpy(&size, xb + pos, sizeof(int)); + else + size = 0;
The else here is effectively an error break with the test for size > 0 in the while loop? Why not remove the size > 0 test and just do a break here? Probably reverse the test too: if (pos >= xb_size - sizeof(int)) break; memcpy(&size, xb + pos, sizeof(int));
+ }
+ if (exist_size)
+ *exist_size = 0;
+ return -ENODATA;
+}
+
+static int nval_used(const char *xb, int xb_size)
+{
+ int pos = 0;
+ int size;
+
+ memcpy(&size, xb + pos, sizeof(int));
+ while (size > 0 && (size < xb_size) && (pos + size < xb_size)) {
+ pos += size;
+ if (pos < xb_size - sizeof(int))
+ memcpy(&size, xb + pos, sizeof(int));
+ else
+ size = 0;Same here, remove the size > 0 test and just break from the loop.
+ }
+ return pos;
+}
+
+int nval_del(char *xb, int xb_size, const YCHAR *name)
+{
+ int pos = nval_find(xb, xb_size, name, NULL);
+ int size;
+
+ if (pos < 0 || pos >= xb_size)
+ return -ENODATA;
+
+ /* Find size, shift rest over this record,
+ * then zero out the rest of buffer */
+ memcpy(&size, xb + pos, sizeof(int));
+ memcpy(xb + pos, xb + pos + size, xb_size - (pos + size));
+ memset(xb + (xb_size - size), 0, size);
+ return 0;
+}
+
+int nval_set(char *xb, int xb_size, const YCHAR *name, const char *buf,
+ int bsize, int flags)
+{
+ int pos;
+ int namelen = strnlen(name, xb_size);
+ int reclen;
+ int size_exist = 0;
+ int space;
+ int start;These int definitions could all go on one line.
+
+ pos = nval_find(xb, xb_size, name, &size_exist);
+
+ if (flags & XATTR_CREATE && pos >= 0)
+ return -EEXIST;
+ if (flags & XATTR_REPLACE && pos < 0)
+ return -ENODATA;
+
+ start = nval_used(xb, xb_size);
+ space = xb_size - start + size_exist;
+
+ reclen = (sizeof(int) + namelen + 1 + bsize);
+
+ if (reclen > space)
+ return -ENOSPC;
+
+ if (pos >= 0) {
+ nval_del(xb, xb_size, name);
+ start = nval_used(xb, xb_size);
+ }
+
+ pos = start;
+
+ memcpy(xb + pos, &reclen, sizeof(int));
+ pos += sizeof(int);
+ strncpy((YCHAR *) (xb + pos), name, reclen);
+ pos += (namelen + 1);
+ memcpy(xb + pos, buf, bsize);
static const char *nval_value(const char *xb, int pos, int namelen)
{
return xb + sizeof(int) + namelen + 1;
}
With the above helper replace the above with:
memcpy(xb + pos, &reclen, sizeof(int));
strncpy(nval_name(xb, pos), name, namelen);
strncpy(nval_value(xb, pos, namelen), buf, bsize);
pos += reclen;
Note the change from reclen to namelen in the first strncpy. I think
reclen is incorrect here?
+ return 0;
+}
+
+int nval_get(const char *xb, int xb_size, const YCHAR * name, char *buf,
+ int bsize)
+{
+ int pos = nval_find(xb, xb_size, name, NULL);
+ int size;
+
+ if (pos >= 0 && pos < xb_size) {if (pos >= xb_size) return -ERANGE; if (pos < 0) return -ENODATA; Then drop the indentation for the code below.
+
+ memcpy(&size, xb + pos, sizeof(int));
+ pos += sizeof(int); /* advance past record length */
+ size -= sizeof(int);
+
+ /* Advance over name string */
+ while (xb[pos] && size > 0 && pos < xb_size) {
+ pos++;
+ size--;
+ }
+ /*Advance over NUL */
+ pos++;
+ size--;
+
+ if (size <= bsize) {
+ memcpy(buf, xb + pos, size);
+ return size;
+ }
+ }
+ if (pos >= 0)
+ return -ERANGE;
+
+ return -ENODATA;
+}
+
+int nval_list(const char *xb, int xb_size, char *buf, int bsize)
+{
+ int pos = 0;
+ int size;
+ int name_len;
+ int ncopied = 0;
+ int filled = 0;These can all go on one line.
+
+ memcpy(&size, xb + pos, sizeof(int));
+ while (size > sizeof(int) &&
+ size <= xb_size &&
+ (pos + size) < xb_size &&
+ !filled) {
+ pos += sizeof(int);
+ size -= sizeof(int);
+ name_len = strnlen((YCHAR *) (xb + pos), size);name_len = strnlen(nval_name(xb, pos), size);
+ if (ncopied + name_len + 1 < bsize) {
+ memcpy(buf, xb + pos, name_len * sizeof(YCHAR));
+ buf += name_len;
+ *buf = '\0';
+ buf++;
+ if (sizeof(YCHAR) > 1) {
+ *buf = '\0';
+ buf++;
+ }This could be simplified as: memset(buf, 0, sizeof(YCHAR)); buf += sizeof(YCHAR);
quoted hunk ↗ jump to hunk
+ ncopied += (name_len + 1); + } else { + filled = 1; + } + pos += size; + if (pos < xb_size - sizeof(int)) + memcpy(&size, xb + pos, sizeof(int)); + else + size = 0; + } + return ncopied; +} + +int nval_hasvalues(const char *xb, int xb_size) +{ + return nval_used(xb, xb_size) > 0; +}diff --git a/fs/yaffs2/yaffs_nameval.h b/fs/yaffs2/yaffs_nameval.h new file mode 100644 index 0000000..951e64f --- /dev/null +++ b/fs/yaffs2/yaffs_nameval.h@@ -0,0 +1,28 @@ +/* + * YAFFS: Yet another Flash File System . A NAND-flash specific file system. + * + * Copyright (C) 2002-2011 Aleph One Ltd. + * for Toby Churchill Ltd and Brightstar Engineering + * + * Created by Charles Manning <charles@aleph1.co.uk> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License version 2.1 as + * published by the Free Software Foundation. + * + * Note: Only YAFFS headers are LGPL, YAFFS C code is covered by GPL. + */ + +#ifndef __NAMEVAL_H__ +#define __NAMEVAL_H__ + +#include "yportenv.h" + +int nval_del(char *xb, int xb_size, const YCHAR * name); +int nval_set(char *xb, int xb_size, const YCHAR * name, const char *buf, + int bsize, int flags); +int nval_get(const char *xb, int xb_size, const YCHAR * name, char *buf, + int bsize); +int nval_list(const char *xb, int xb_size, char *buf, int bsize); +int nval_hasvalues(const char *xb, int xb_size); +#endif
-- Bluewater Systems Ltd - ARM Technology Solution Centre Ryan Mallon 5 Amuri Park, 404 Barbadoes St ryan@bluewatersys.com PO Box 13 889, Christchurch 8013 http://www.bluewatersys.com New Zealand Phone: +64 3 3779127 Freecall: Australia 1800 148 751 Fax: +64 3 3779135 USA 1800 261 2934