[{"id":1762279,"web_url":"http://patchwork.ozlabs.org/comment/1762279/","msgid":"<660af9d7-5bb7-997e-7d0f-8381046f62dc@denx.de>","list_archive_url":null,"date":"2017-09-03T15:08:05","subject":"Re: [U-Boot] [PATCH v2 2/8] fs/fat: introduce new director iterators","submitter":{"id":70701,"url":"http://patchwork.ozlabs.org/api/people/70701/","name":"Lukasz Majewski","email":"lukma@denx.de"},"content":"On 09/02/2017 06:37 PM, Rob Clark wrote:\n> Untangle directory traversal into a simple iterator, to replace the\n> existing multi-purpose do_fat_read_at() + get_dentfromdir().\n> \n> Signed-off-by: Rob Clark <robdclark@gmail.com>\n\nReviewed-by: Łukasz Majewski <lukma@denx.de>\n\n> ---\n>   fs/fat/fat.c | 326 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n>   1 file changed, 326 insertions(+)\n> \n> diff --git a/fs/fat/fat.c b/fs/fat/fat.c\n> index e1c0a15dc7..c72d6ca931 100644\n> --- a/fs/fat/fat.c\n> +++ b/fs/fat/fat.c\n> @@ -1245,6 +1245,332 @@ exit:\n>   \treturn ret;\n>   }\n>   \n> +\n> +/*\n> + * Directory iterator, to simplify filesystem traversal\n> + */\n> +\n> +typedef struct {\n> +\tfsdata    *fsdata;\n> +\tunsigned   cursect;\n> +\tdir_entry *dent;\n> +\tint        remaining;     /* remaining dent's in current cluster */\n> +\tint        last_cluster;\n> +\tint        is_root;\n> +\n> +\t/* current iterator position values: */\n> +\tchar       l_name[VFAT_MAXLEN_BYTES];\n> +\tchar       s_name[14];\n> +\tchar      *name;          /* l_name if there is one, else s_name */\n> +\n> +\tu8         block[MAX_CLUSTSIZE] __aligned(ARCH_DMA_MINALIGN);\n> +} fat_itr;\n> +\n> +static int fat_itr_isdir(fat_itr *itr);\n> +\n> +/**\n> + * fat_itr_root() - initialize an iterator to start at the root\n> + * directory\n> + *\n> + * @itr: iterator to initialize\n> + * @fsdata: filesystem data for the partition\n> + * @return 0 on success, else -errno\n> + */\n> +static int fat_itr_root(fat_itr *itr, fsdata *fsdata)\n> +{\n> +\tif (get_fs_info(fsdata))\n> +\t\treturn -ENXIO;\n> +\n> +\titr->fsdata = fsdata;\n> +\titr->cursect = fsdata->rootdir_sect;\n> +\titr->dent = NULL;\n> +\titr->remaining = 0;\n> +\titr->last_cluster = 0;\n> +\titr->is_root = 1;\n> +\n> +\treturn 0;\n> +}\n> +\n> +/**\n> + * fat_itr_child() - initialize an iterator to descend into a sub-\n> + * directory\n> + *\n> + * Initializes 'itr' to iterate the contents of the directory at\n> + * the current cursor position of 'parent'.  It is an error to\n> + * call this if the current cursor of 'parent' is pointing at a\n> + * regular file.\n> + *\n> + * Note that 'itr' and 'parent' can be the same pointer if you do\n> + * not need to preserve 'parent' after this call, which is useful\n> + * for traversing directory structure to resolve a file/directory.\n> + *\n> + * @itr: iterator to initialize\n> + * @parent: the iterator pointing at a directory entry in the\n> + *    parent directory of the directory to iterate\n> + */\n> +static void fat_itr_child(fat_itr *itr, fat_itr *parent)\n> +{\n> +\tfsdata *mydata = parent->fsdata;  /* for silly macros */\n> +\tunsigned clustnum = START(parent->dent);\n> +\n> +\tassert(fat_itr_isdir(parent));\n> +\n> +\titr->fsdata = parent->fsdata;\n> +\tif (clustnum > 0) {\n> +\t\titr->cursect = itr->fsdata->data_begin +\n> +\t\t\t(clustnum * itr->fsdata->clust_size);\n> +\t} else {\n> +\t\titr->cursect = parent->fsdata->rootdir_sect;\n> +\t}\n> +\titr->dent = NULL;\n> +\titr->remaining = 0;\n> +\titr->last_cluster = 0;\n> +\titr->is_root = 0;\n> +}\n> +\n> +static void *next_cluster(fat_itr *itr)\n> +{\n> +\tfsdata *mydata = itr->fsdata;  /* for silly macros */\n> +\tint ret;\n> +\n> +\t/* have we reached the end? */\n> +\tif (itr->last_cluster)\n> +\t\treturn NULL;\n> +\n> +\tdebug(\"FAT read(sect=%d), clust_size=%d, DIRENTSPERBLOCK=%zd\\n\",\n> +\t      itr->cursect, itr->fsdata->clust_size, DIRENTSPERBLOCK);\n> +\n> +\t/*\n> +\t * NOTE: do_fat_read_at() had complicated logic to deal w/\n> +\t * vfat names that span multiple clusters in the fat16 case,\n> +\t * which get_dentfromdir() probably also needed (and was\n> +\t * missing).  And not entirely sure what fat32 didn't have\n> +\t * the same issue..  We solve that by only caring about one\n> +\t * dent at a time and iteratively constructing the vfat long\n> +\t * name.\n> +\t */\n> +\tret = disk_read(itr->cursect, itr->fsdata->clust_size,\n> +\t\t\titr->block);\n> +\tif (ret < 0) {\n> +\t\tdebug(\"Error: reading block\\n\");\n> +\t\treturn NULL;\n> +\t}\n> +\n> +\tif (itr->is_root && itr->fsdata->fatsize != 32) {\n> +\t\titr->cursect++;\n> +\t\tif (itr->cursect - itr->fsdata->rootdir_sect >=\n> +\t\t    itr->fsdata->rootdir_size) {\n> +\t\t\tdebug(\"cursect: 0x%x\\n\", itr->cursect);\n> +\t\t\titr->last_cluster = 1;\n> +\t\t}\n> +\t} else {\n> +\t\titr->cursect = get_fatent(itr->fsdata, itr->cursect);\n> +\t\tif (CHECK_CLUST(itr->cursect, itr->fsdata->fatsize)) {\n> +\t\t\tdebug(\"cursect: 0x%x\\n\", itr->cursect);\n> +\t\t\titr->last_cluster = 1;\n> +\t\t}\n> +\t}\n> +\n> +\treturn itr->block;\n> +}\n> +\n> +static dir_entry *next_dent(fat_itr *itr)\n> +{\n> +\tif (itr->remaining == 0) {\n> +\t\tstruct dir_entry *dent = next_cluster(itr);\n> +\n> +\t\t/* have we reached the last cluster? */\n> +\t\tif (!dent)\n> +\t\t\treturn NULL;\n> +\n> +\t\titr->remaining = itr->fsdata->sect_size / sizeof(dir_entry) - 1;\n> +\t\titr->dent = dent;\n> +\t} else {\n> +\t\titr->remaining--;\n> +\t\titr->dent++;\n> +\t}\n> +\n> +\t/* have we reached the last valid entry? */\n> +\tif (itr->dent->name[0] == 0)\n> +\t\treturn NULL;\n> +\n> +\treturn itr->dent;\n> +}\n> +\n> +static dir_entry *extract_vfat_name(fat_itr *itr)\n> +{\n> +\tstruct dir_entry *dent = itr->dent;\n> +\tint seqn = itr->dent->name[0] & ~LAST_LONG_ENTRY_MASK;\n> +\tu8 chksum, alias_checksum = ((dir_slot *)dent)->alias_checksum;\n> +\tint n = 0;\n> +\n> +\twhile (seqn--) {\n> +\t\tchar buf[13];\n> +\t\tint idx = 0;\n> +\n> +\t\tslot2str((dir_slot *)dent, buf, &idx);\n> +\n> +\t\t/* shift accumulated long-name up and copy new part in: */\n> +\t\tmemmove(itr->l_name + idx, itr->l_name, n);\n> +\t\tmemcpy(itr->l_name, buf, idx);\n> +\t\tn += idx;\n> +\n> +\t\tdent = next_dent(itr);\n> +\t\tif (!dent)\n> +\t\t\treturn NULL;\n> +\t}\n> +\n> +\titr->l_name[n] = '\\0';\n> +\n> +\tchksum = mkcksum(dent->name, dent->ext);\n> +\n> +\t/* checksum mismatch could mean deleted file, etc.. skip it: */\n> +\tif (chksum != alias_checksum) {\n> +\t\tdebug(\"** chksum=%x, alias_checksum=%x, l_name=%s, s_name=%8s.%3s\\n\",\n> +\t\t      chksum, alias_checksum, itr->l_name, dent->name, dent->ext);\n> +\t\treturn NULL;\n> +\t}\n> +\n> +\treturn dent;\n> +}\n> +\n> +/**\n> + * fat_itr_next() - step to the next entry in a directory\n> + *\n> + * Must be called once on a new iterator before the cursor is valid.\n> + *\n> + * @itr: the iterator to iterate\n> + * @return boolean, 1 if success or 0 if no more entries in the\n> + *    current directory\n> + */\n> +static int fat_itr_next(fat_itr *itr)\n> +{\n> +\tdir_entry *dent;\n> +\n> +\titr->name = NULL;\n> +\n> +\twhile (1) {\n> +\t\tdent = next_dent(itr);\n> +\t\tif (!dent)\n> +\t\t\treturn 0;\n> +\n> +\t\tif (dent->name[0] == DELETED_FLAG ||\n> +\t\t    dent->name[0] == aRING)\n> +\t\t\tcontinue;\n> +\n> +\t\tif (dent->attr & ATTR_VOLUME) {\n> +\t\t\tif (vfat_enabled &&\n> +\t\t\t    (dent->attr & ATTR_VFAT) == ATTR_VFAT &&\n> +\t\t\t    (dent->name[0] & LAST_LONG_ENTRY_MASK)) {\n> +\t\t\t\tdent = extract_vfat_name(itr);\n> +\t\t\t\tif (!dent)\n> +\t\t\t\t\tcontinue;\n> +\t\t\t\titr->name = itr->l_name;\n> +\t\t\t\tbreak;\n> +\t\t\t} else {\n> +\t\t\t\t/* Volume label or VFAT entry, skip */\n> +\t\t\t\tcontinue;\n> +\t\t\t}\n> +\t\t}\n> +\n> +\t\tbreak;\n> +\t}\n> +\n> +\tget_name(dent, itr->s_name);\n> +\tif (!itr->name)\n> +\t\titr->name = itr->s_name;\n> +\n> +\treturn 1;\n> +}\n> +\n> +/**\n> + * fat_itr_isdir() - is current cursor position pointing to a directory\n> + *\n> + * @itr: the iterator\n> + * @return true if cursor is at a directory\n> + */\n> +static int fat_itr_isdir(fat_itr *itr)\n> +{\n> +\treturn !!(itr->dent->attr & ATTR_DIR);\n> +}\n> +\n> +/*\n> + * Helpers:\n> + */\n> +\n> +#define TYPE_FILE 0x1\n> +#define TYPE_DIR  0x2\n> +#define TYPE_ANY  (TYPE_FILE | TYPE_DIR)\n> +\n> +/**\n> + * fat_itr_resolve() - traverse directory structure to resolve the\n> + * requested path.\n> + *\n> + * Traverse directory structure to the requested path.  If the specified\n> + * path is to a directory, this will descend into the directory and\n> + * leave it iterator at the start of the directory.  If the path is to a\n> + * file, it will leave the iterator in the parent directory with current\n> + * cursor at file's entry in the directory.\n> + *\n> + * @itr: iterator initialized to root\n> + * @path: the requested path\n> + * @type: bitmask of allowable file types\n> + * @return 0 on success or -errno\n> + */\n> +static int fat_itr_resolve(fat_itr *itr, const char *path, unsigned type)\n> +{\n> +\tconst char *next;\n> +\n> +\t/* chomp any extra leading slashes: */\n> +\twhile (path[0] && ISDIRDELIM(path[0]))\n> +\t\tpath++;\n> +\n> +\t/* are we at the end? */\n> +\tif (strlen(path) == 0) {\n> +\t\tif (!(type & TYPE_DIR))\n> +\t\t\treturn -ENOENT;\n> +\t\treturn 0;\n> +\t}\n> +\n> +\t/* find length of next path entry: */\n> +\tnext = path;\n> +\twhile (next[0] && !ISDIRDELIM(next[0]))\n> +\t\tnext++;\n> +\n> +\twhile (fat_itr_next(itr)) {\n> +\t\tint match = 0;\n> +\n> +\t\t/* check both long and short name: */\n> +\t\tif (!strncasecmp(path, itr->name, next - path))\n> +\t\t\tmatch = 1;\n> +\t\telse if (itr->name != itr->s_name &&\n> +\t\t\t !strncasecmp(path, itr->s_name, (next - path)))\n> +\t\t\tmatch = 1;\n> +\n> +\t\tif (!match)\n> +\t\t\tcontinue;\n> +\n> +\t\tif (fat_itr_isdir(itr)) {\n> +\t\t\t/* recurse into directory: */\n> +\t\t\tfat_itr_child(itr, itr);\n> +\t\t\treturn fat_itr_resolve(itr, next, type);\n> +\t\t} else if (next[0]) {\n> +\t\t\t/*\n> +\t\t\t * If next is not empty then we have a case\n> +\t\t\t * like: /path/to/realfile/nonsense\n> +\t\t\t */\n> +\t\t\tdebug(\"bad trailing path: %s\\n\", next);\n> +\t\t\treturn -ENOENT;\n> +\t\t} else if (!(type & TYPE_FILE)) {\n> +\t\t\treturn -ENOTDIR;\n> +\t\t} else {\n> +\t\t\treturn 0;\n> +\t\t}\n> +\t}\n> +\n> +\treturn -ENOENT;\n> +}\n> +\n>   int do_fat_read(const char *filename, void *buffer, loff_t maxsize, int dols,\n>   \t\tloff_t *actread)\n>   {\n>","headers":{"Return-Path":"<u-boot-bounces@lists.denx.de>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.denx.de\n\t(client-ip=81.169.180.215; helo=lists.denx.de;\n\tenvelope-from=u-boot-bounces@lists.denx.de;\n\treceiver=<UNKNOWN>)","Received":["from lists.denx.de (dione.denx.de [81.169.180.215])\n\tby ozlabs.org (Postfix) with ESMTP id 3xlbw23W6Rz9t2y\n\tfor <incoming@patchwork.ozlabs.org>;\n\tMon,  4 Sep 2017 01:08:34 +1000 (AEST)","by lists.denx.de (Postfix, from userid 105)\n\tid 19DB0C21E6F; Sun,  3 Sep 2017 15:08:29 +0000 (UTC)","from lists.denx.de (localhost [IPv6:::1])\n\tby lists.denx.de (Postfix) with ESMTP id 24F0AC21E30;\n\tSun,  3 Sep 2017 15:08:26 +0000 (UTC)","by lists.denx.de (Postfix, from userid 105)\n\tid 885D1C21E6F; Sun,  3 Sep 2017 15:08:07 +0000 (UTC)","from mail-out.m-online.net (mail-out.m-online.net [212.18.0.10])\n\tby lists.denx.de (Postfix) with ESMTPS id AB88EC21F18\n\tfor <u-boot@lists.denx.de>; Sun,  3 Sep 2017 15:08:07 +0000 (UTC)","from frontend01.mail.m-online.net (unknown [192.168.8.182])\n\tby mail-out.m-online.net (Postfix) with ESMTP id 3xlbvW3WR2z1qw7g;\n\tSun,  3 Sep 2017 17:08:07 +0200 (CEST)","from localhost (dynscan1.mnet-online.de [192.168.6.70])\n\tby mail.m-online.net (Postfix) with ESMTP id 3xlbvW26G6z3hjkw;\n\tSun,  3 Sep 2017 17:08:07 +0200 (CEST)","from mail.mnet-online.de ([192.168.8.182])\n\tby localhost (dynscan1.mail.m-online.net [192.168.6.70]) (amavisd-new,\n\tport 10024)\n\twith ESMTP id lLavUMhD0z_k; Sun,  3 Sep 2017 17:08:06 +0200 (CEST)","from [192.168.2.222] (89-77-92-62.dynamic.chello.pl [89.77.92.62])\n\t(using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mail.mnet-online.de (Postfix) with ESMTPSA;\n\tSun,  3 Sep 2017 17:08:06 +0200 (CEST)"],"X-Spam-Checker-Version":"SpamAssassin 3.4.0 (2014-02-07) on lists.denx.de","X-Spam-Level":"","X-Spam-Status":"No, score=-0.7 required=5.0 tests=RCVD_IN_DNSWL_LOW,\n\tRCVD_IN_MSPIKE_H3,\n\tRCVD_IN_MSPIKE_WL autolearn=unavailable autolearn_force=no\n\tversion=3.4.0","X-Virus-Scanned":"amavisd-new at mnet-online.de","X-Auth-Info":"BFK4oCTcjxV5Wu4E4QzEIeHDZAhEU9MqSzKofq/LJZo=","To":"Rob Clark <robdclark@gmail.com>,\n\tU-Boot Mailing List <u-boot@lists.denx.de>","References":"<20170902163806.27265-1-robdclark@gmail.com>\n\t<20170902163806.27265-3-robdclark@gmail.com>","From":"=?utf-8?q?=C5=81ukasz_Majewski?= <lukma@denx.de>","Organization":"DENX","Message-ID":"<660af9d7-5bb7-997e-7d0f-8381046f62dc@denx.de>","Date":"Sun, 3 Sep 2017 17:08:05 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"<20170902163806.27265-3-robdclark@gmail.com>","Content-Language":"en-US","Subject":"Re: [U-Boot] [PATCH v2 2/8] fs/fat: introduce new director iterators","X-BeenThere":"u-boot@lists.denx.de","X-Mailman-Version":"2.1.18","Precedence":"list","List-Id":"U-Boot discussion <u-boot.lists.denx.de>","List-Unsubscribe":"<https://lists.denx.de/options/u-boot>,\n\t<mailto:u-boot-request@lists.denx.de?subject=unsubscribe>","List-Archive":"<http://lists.denx.de/pipermail/u-boot/>","List-Post":"<mailto:u-boot@lists.denx.de>","List-Help":"<mailto:u-boot-request@lists.denx.de?subject=help>","List-Subscribe":"<https://lists.denx.de/listinfo/u-boot>,\n\t<mailto:u-boot-request@lists.denx.de?subject=subscribe>","Content-Transfer-Encoding":"base64","Content-Type":"text/plain; charset=\"utf-8\"; Format=\"flowed\"","Errors-To":"u-boot-bounces@lists.denx.de","Sender":"\"U-Boot\" <u-boot-bounces@lists.denx.de>"}},{"id":1763092,"web_url":"http://patchwork.ozlabs.org/comment/1763092/","msgid":"<CAPnjgZ1vNYbVL7qvOUjJzNBdO0F40qvdHCOhAiYruRwM2jsfOA@mail.gmail.com>","list_archive_url":null,"date":"2017-09-05T08:56:04","subject":"Re: [U-Boot] [PATCH v2 2/8] fs/fat: introduce new director iterators","submitter":{"id":6170,"url":"http://patchwork.ozlabs.org/api/people/6170/","name":"Simon Glass","email":"sjg@chromium.org"},"content":"Hi Rob,\n\nOn 3 September 2017 at 00:37, Rob Clark <robdclark@gmail.com> wrote:\n> Untangle directory traversal into a simple iterator, to replace the\n> existing multi-purpose do_fat_read_at() + get_dentfromdir().\n>\n> Signed-off-by: Rob Clark <robdclark@gmail.com>\n> ---\n>  fs/fat/fat.c | 326 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n>  1 file changed, 326 insertions(+)\n>\n> diff --git a/fs/fat/fat.c b/fs/fat/fat.c\n> index e1c0a15dc7..c72d6ca931 100644\n> --- a/fs/fat/fat.c\n> +++ b/fs/fat/fat.c\n> @@ -1245,6 +1245,332 @@ exit:\n>         return ret;\n>  }\n>\n> +\n> +/*\n> + * Directory iterator, to simplify filesystem traversal\n> + */\n> +\n> +typedef struct {\n\nPlease avoid using a typedef here. It seems like it could just be a struct.\n\nAlso pleaee add a comment about what the struct is for\n\n> +       fsdata    *fsdata;\n> +       unsigned   cursect;\n> +       dir_entry *dent;\n> +       int        remaining;     /* remaining dent's in current cluster */\n> +       int        last_cluster;\n> +       int        is_root;\n> +\n> +       /* current iterator position values: */\n> +       char       l_name[VFAT_MAXLEN_BYTES];\n> +       char       s_name[14];\n> +       char      *name;          /* l_name if there is one, else s_name */\n> +\n> +       u8         block[MAX_CLUSTSIZE] __aligned(ARCH_DMA_MINALIGN);\n\nSome members are missing comments.\n\nAlso I'm not too sure how the alignment works here. I don't see you\ncreating one of these objects in this patch, but could you add a\ncomment to the struct about how to create one? Do you use memalign()?","headers":{"Return-Path":"<u-boot-bounces@lists.denx.de>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.denx.de\n\t(client-ip=81.169.180.215; helo=lists.denx.de;\n\tenvelope-from=u-boot-bounces@lists.denx.de;\n\treceiver=<UNKNOWN>)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=google.com header.i=@google.com\n\theader.b=\"rUIwYxFX\"; \n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"nOi6LGDV\"; dkim-atps=neutral"],"Received":["from lists.denx.de (dione.denx.de [81.169.180.215])\n\tby ozlabs.org (Postfix) with ESMTP id 3xmggy69mPz9s72\n\tfor <incoming@patchwork.ozlabs.org>;\n\tTue,  5 Sep 2017 19:01:50 +1000 (AEST)","by lists.denx.de (Postfix, from userid 105)\n\tid 87791C21EFE; Tue,  5 Sep 2017 08:58:14 +0000 (UTC)","from lists.denx.de (localhost [IPv6:::1])\n\tby lists.denx.de (Postfix) with ESMTP id 5AF6DC21C97;\n\tTue,  5 Sep 2017 08:57:30 +0000 (UTC)","by lists.denx.de (Postfix, from userid 105)\n\tid A0A00C21C45; Tue,  5 Sep 2017 08:56:29 +0000 (UTC)","from mail-qt0-f174.google.com (mail-qt0-f174.google.com\n\t[209.85.216.174])\n\tby lists.denx.de (Postfix) with ESMTPS id 74F1EC21EE2\n\tfor <u-boot@lists.denx.de>; Tue,  5 Sep 2017 08:56:26 +0000 (UTC)","by mail-qt0-f174.google.com with SMTP id h15so9754634qta.4\n\tfor <u-boot@lists.denx.de>; Tue, 05 Sep 2017 01:56:26 -0700 (PDT)","by 10.200.28.108 with HTTP; Tue, 5 Sep 2017 01:56:04 -0700 (PDT)"],"X-Spam-Checker-Version":"SpamAssassin 3.4.0 (2014-02-07) on lists.denx.de","X-Spam-Level":"","X-Spam-Status":"No, score=-0.0 required=5.0 tests=RCVD_IN_DNSWL_NONE,\n\tRCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL,\n\tT_DKIM_INVALID autolearn=unavailable\n\tautolearn_force=no version=3.4.0","DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com;\n\ts=20161025; \n\th=mime-version:sender:in-reply-to:references:from:date:message-id\n\t:subject:to:cc;\n\tbh=J+UoVjP1qcsGbg5bIqfI7487HRpOnhSHfzhOnRQGCJo=;\n\tb=rUIwYxFXaAtHFvVocgJylYBExQiF21GaIEKl8murshfQuaFc9R3Z8h9BugmjMmweQE\n\t8eM9RwqHK2YjFi6cGx+8l68Ag8sxXFaiTcoJexSxorrPa60pKhRwq6vhiQJeul8/SIgN\n\txv5DwTaRUhd+5QFHa7EsJWGXPeysxJPG1uIy6+lk073Q/RM5WBPegnLp68N4Ir4RJg1G\n\twK3gF6cUBPh/qDTXR3EoBdW5U2giTZouCofG7UnFZvrOoAuAp0tUdNgfM+5fIiskRXYM\n\t5w9SrqpBZ2/5NO0Kvlk2DbPh4GT8aYeK8MD+4jKUBsM1q+vWlg2es3+m5Xo36SWui+Ls\n\tlsSA==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:sender:in-reply-to:references:from:date:message-id\n\t:subject:to:cc;\n\tbh=J+UoVjP1qcsGbg5bIqfI7487HRpOnhSHfzhOnRQGCJo=;\n\tb=nOi6LGDVarXS5AniHxpV1lov0iJboY1ng3Wg+SDtmVXZeBchNGmACHGjQwqwvB+tw2\n\tKhNI7drvGvlFtxGhRH1nHlKilMY0tAZQ7EIc3yXcjrd4EXe7KWW05NHeVAiEKX3gROOk\n\tpZqx2W1BxJuq1OaxvTCjrXo9McU5lN1F2CS/M="],"X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:sender:in-reply-to:references:from\n\t:date:message-id:subject:to:cc;\n\tbh=J+UoVjP1qcsGbg5bIqfI7487HRpOnhSHfzhOnRQGCJo=;\n\tb=feyXLMEhdcQLrcJZHWc/o7yzDm0uJf8MNTYXoD2eRE6a23hrDObw6lAsuVDehfypTh\n\t948xIWk1Xl2yt2C3+NTzIjIgkIX7zJtiIIIoX3qckrHqlmR5oZ2Y1HCEZ6dhFl1FjB5Y\n\tzVDs87I01idmk/IT4Or+5QeSYtjJQjqDmwj8igu0PKlsNQsJ7U1hR3f91ciPtJk0cb0n\n\t193ElA0/sh/e5TBqbGhMZpZadiDUUpTq0tJ3mrwjxZjUMIvnmfChOJPbH01aKJ7kR8Hc\n\tjj/2MPKqrU672QOcOWVBn9zmQSWswv4MeStzZS9aVOOMNLxzrRsKM7qt2gKV9f5K5q5C\n\todAA==","X-Gm-Message-State":"AHPjjUhd1mpPrLcVWXzFgFbGOCAnoXVj45LINHuCGVdLC+JAvqoqsRcC\n\tIfx2C2RUDiowdTQ2Hjg+72WBCprbTe9v","X-Google-Smtp-Source":"ADKCNb5bU6JgsfK/HvfDzkGWG59LRT66HKWnb+7DbIO6FRTpW1EkvSod9vv9EVVXY6e+aCUqoSMiY9dwXx7umbCRdA4=","X-Received":"by 10.200.44.198 with SMTP id 6mr4566218qtx.68.1504601785226;\n\tTue, 05 Sep 2017 01:56:25 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"<20170902163806.27265-3-robdclark@gmail.com>","References":"<20170902163806.27265-1-robdclark@gmail.com>\n\t<20170902163806.27265-3-robdclark@gmail.com>","From":"Simon Glass <sjg@chromium.org>","Date":"Tue, 5 Sep 2017 16:56:04 +0800","X-Google-Sender-Auth":"OyI9Mh9rGgJpypL81j3NunMEeGY","Message-ID":"<CAPnjgZ1vNYbVL7qvOUjJzNBdO0F40qvdHCOhAiYruRwM2jsfOA@mail.gmail.com>","To":"Rob Clark <robdclark@gmail.com>","Cc":"U-Boot Mailing List <u-boot@lists.denx.de>","Subject":"Re: [U-Boot] [PATCH v2 2/8] fs/fat: introduce new director iterators","X-BeenThere":"u-boot@lists.denx.de","X-Mailman-Version":"2.1.18","Precedence":"list","List-Id":"U-Boot discussion <u-boot.lists.denx.de>","List-Unsubscribe":"<https://lists.denx.de/options/u-boot>,\n\t<mailto:u-boot-request@lists.denx.de?subject=unsubscribe>","List-Archive":"<http://lists.denx.de/pipermail/u-boot/>","List-Post":"<mailto:u-boot@lists.denx.de>","List-Help":"<mailto:u-boot-request@lists.denx.de?subject=help>","List-Subscribe":"<https://lists.denx.de/listinfo/u-boot>,\n\t<mailto:u-boot-request@lists.denx.de?subject=subscribe>","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"u-boot-bounces@lists.denx.de","Sender":"\"U-Boot\" <u-boot-bounces@lists.denx.de>"}},{"id":1763163,"web_url":"http://patchwork.ozlabs.org/comment/1763163/","msgid":"<CAF6AEGuoT=XhgBZKjzxqmjgRKXpto+nZXrz=XfixqVYzVYF4Ww@mail.gmail.com>","list_archive_url":null,"date":"2017-09-05T09:54:07","subject":"Re: [U-Boot] [PATCH v2 2/8] fs/fat: introduce new director iterators","submitter":{"id":18760,"url":"http://patchwork.ozlabs.org/api/people/18760/","name":"Rob Clark","email":"robdclark@gmail.com"},"content":"On Tue, Sep 5, 2017 at 4:56 AM, Simon Glass <sjg@chromium.org> wrote:\n> Hi Rob,\n>\n> On 3 September 2017 at 00:37, Rob Clark <robdclark@gmail.com> wrote:\n>> Untangle directory traversal into a simple iterator, to replace the\n>> existing multi-purpose do_fat_read_at() + get_dentfromdir().\n>>\n>> Signed-off-by: Rob Clark <robdclark@gmail.com>\n>> ---\n>>  fs/fat/fat.c | 326 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n>>  1 file changed, 326 insertions(+)\n>>\n>> diff --git a/fs/fat/fat.c b/fs/fat/fat.c\n>> index e1c0a15dc7..c72d6ca931 100644\n>> --- a/fs/fat/fat.c\n>> +++ b/fs/fat/fat.c\n>> @@ -1245,6 +1245,332 @@ exit:\n>>         return ret;\n>>  }\n>>\n>> +\n>> +/*\n>> + * Directory iterator, to simplify filesystem traversal\n>> + */\n>> +\n>> +typedef struct {\n>\n> Please avoid using a typedef here. It seems like it could just be a struct.\n\nI'm typically not a fan of 'typedef struct' but that seems to be the\nstyle that fs/fat code was already using, and \"when in Rome...\"\n\n> Also pleaee add a comment about what the struct is for\n>\n>> +       fsdata    *fsdata;\n>> +       unsigned   cursect;\n>> +       dir_entry *dent;\n>> +       int        remaining;     /* remaining dent's in current cluster */\n>> +       int        last_cluster;\n>> +       int        is_root;\n>> +\n>> +       /* current iterator position values: */\n>> +       char       l_name[VFAT_MAXLEN_BYTES];\n>> +       char       s_name[14];\n>> +       char      *name;          /* l_name if there is one, else s_name */\n>> +\n>> +       u8         block[MAX_CLUSTSIZE] __aligned(ARCH_DMA_MINALIGN);\n>\n> Some members are missing comments.\n>\n> Also I'm not too sure how the alignment works here. I don't see you\n> creating one of these objects in this patch, but could you add a\n> comment to the struct about how to create one? Do you use memalign()?\n\nI was just creating them on the stack normally.. expecting the\ncompiler to dtrt because of the __aligned() on block[].\n\nBR,\n-R","headers":{"Return-Path":"<u-boot-bounces@lists.denx.de>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.denx.de\n\t(client-ip=81.169.180.215; helo=lists.denx.de;\n\tenvelope-from=u-boot-bounces@lists.denx.de;\n\treceiver=<UNKNOWN>)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"opz7qNtm\"; dkim-atps=neutral"],"Received":["from lists.denx.de (dione.denx.de [81.169.180.215])\n\tby ozlabs.org (Postfix) with ESMTP id 3xmhs76x8Vz9s9Y\n\tfor <incoming@patchwork.ozlabs.org>;\n\tTue,  5 Sep 2017 19:54:51 +1000 (AEST)","by lists.denx.de (Postfix, from userid 105)\n\tid AF8AFC21F6C; Tue,  5 Sep 2017 09:54:42 +0000 (UTC)","from lists.denx.de (localhost [IPv6:::1])\n\tby lists.denx.de (Postfix) with ESMTP id 3A2EDC21DAB;\n\tTue,  5 Sep 2017 09:54:40 +0000 (UTC)","by lists.denx.de (Postfix, from userid 105)\n\tid 87F5AC21C45; Tue,  5 Sep 2017 09:54:09 +0000 (UTC)","from mail-lf0-f53.google.com (mail-lf0-f53.google.com\n\t[209.85.215.53])\n\tby lists.denx.de (Postfix) with ESMTPS id C12C8C21FB8\n\tfor <u-boot@lists.denx.de>; Tue,  5 Sep 2017 09:54:08 +0000 (UTC)","by mail-lf0-f53.google.com with SMTP id d17so9198181lfe.2\n\tfor <u-boot@lists.denx.de>; Tue, 05 Sep 2017 02:54:08 -0700 (PDT)","by 10.46.82.27 with HTTP; Tue, 5 Sep 2017 02:54:07 -0700 (PDT)"],"X-Spam-Checker-Version":"SpamAssassin 3.4.0 (2014-02-07) on lists.denx.de","X-Spam-Level":"","X-Spam-Status":"No, score=-0.0 required=5.0 tests=FREEMAIL_FROM,\n\tRCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL,\n\tT_DKIM_INVALID\n\tautolearn=unavailable autolearn_force=no version=3.4.0","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;\n\th=mime-version:in-reply-to:references:from:date:message-id:subject:to\n\t:cc; bh=78hiBUDAxFkNbT9eq5U4fEnlg4NGNBY4Xnz/VnsUlUM=;\n\tb=opz7qNtmQ0JoI3I3qMRyyjus8nZrahLF4uslO+zR0YqabWp0oWJojoA5UGYBXiqLZF\n\tV+H9Lohv0MLgJkGdtjV7KHNl6wiDv/rPSFdn6O/MdpUsigugHv7dgP/6Jo5Lz9X46sZw\n\tnLv6kWmzmEr9h7SCaSNS0d8BYbVEKqtP3ac/ghRr4ig5x7adkmuP2hQZH6ukLhv3yupz\n\tUj0RKIDdMjWTQICfV8Id0JRmmIGdvDJUjmsXPtA3jnqBQDOSDCpJdUaSeVnhfXs15tmC\n\tNuSf1abCPADTTyRY1MOHe8plargKiXkvqxvN08529mwnxhYaKKrCyUx4t0gXgmB8KK/A\n\tYx7A==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:in-reply-to:references:from:date\n\t:message-id:subject:to:cc;\n\tbh=78hiBUDAxFkNbT9eq5U4fEnlg4NGNBY4Xnz/VnsUlUM=;\n\tb=BAi0Ebh/ABN6ujPJ8Qhnw2g5vzXRmj728y3vOgOVJ7mm39vPKrId3ArebD+PQOT6YI\n\tpbiLAqzvDbpP6IGGSvjaz9+OSiXH/K7bcY5PvGp1xQA0l3ne/9PDQ4QZUWrjV90zdrcF\n\teiuR9AZoKrOAOPGyFg/a8A46In0UronrNW1ObhBesShP9vL0kIqdG7kvf5qtWUIQXOHb\n\ti7bIzNIC8yjIkK0LZHRVwnZMG4Twz+sxVLsBYi2YZ74sx+ctIeP/pcOnNdR8HjEWVVfD\n\tseqp29uCrxPTezg7MWQeyPtehBdo2gZzRrvTWxRr1uMiXO16ZdPWeZkedBY69+m4c2Gp\n\tP/RQ==","X-Gm-Message-State":"AHPjjUhjDODe1ycPQpncSqgAaKfTJhiakDo/DLl7AaTpLmC5V6ryNMZl\n\tNVjfGKAY/T6Jgc1xwIWzKTGGdsrx9Q==","X-Google-Smtp-Source":"ADKCNb4NEVNL1BU6O4YGKbm8GoN0lOXLitFhVvEOHtnl1zpP2B6ohoTsATlMMveOWUzG/jo/2qLmHJmNAdGSDv4F5bU=","X-Received":"by 10.25.213.71 with SMTP id m68mr983332lfg.214.1504605248210;\n\tTue, 05 Sep 2017 02:54:08 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"<CAPnjgZ1vNYbVL7qvOUjJzNBdO0F40qvdHCOhAiYruRwM2jsfOA@mail.gmail.com>","References":"<20170902163806.27265-1-robdclark@gmail.com>\n\t<20170902163806.27265-3-robdclark@gmail.com>\n\t<CAPnjgZ1vNYbVL7qvOUjJzNBdO0F40qvdHCOhAiYruRwM2jsfOA@mail.gmail.com>","From":"Rob Clark <robdclark@gmail.com>","Date":"Tue, 5 Sep 2017 05:54:07 -0400","Message-ID":"<CAF6AEGuoT=XhgBZKjzxqmjgRKXpto+nZXrz=XfixqVYzVYF4Ww@mail.gmail.com>","To":"Simon Glass <sjg@chromium.org>","Cc":"U-Boot Mailing List <u-boot@lists.denx.de>","Subject":"Re: [U-Boot] [PATCH v2 2/8] fs/fat: introduce new director iterators","X-BeenThere":"u-boot@lists.denx.de","X-Mailman-Version":"2.1.18","Precedence":"list","List-Id":"U-Boot discussion <u-boot.lists.denx.de>","List-Unsubscribe":"<https://lists.denx.de/options/u-boot>,\n\t<mailto:u-boot-request@lists.denx.de?subject=unsubscribe>","List-Archive":"<http://lists.denx.de/pipermail/u-boot/>","List-Post":"<mailto:u-boot@lists.denx.de>","List-Help":"<mailto:u-boot-request@lists.denx.de?subject=help>","List-Subscribe":"<https://lists.denx.de/listinfo/u-boot>,\n\t<mailto:u-boot-request@lists.denx.de?subject=subscribe>","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"u-boot-bounces@lists.denx.de","Sender":"\"U-Boot\" <u-boot-bounces@lists.denx.de>"}},{"id":1765737,"web_url":"http://patchwork.ozlabs.org/comment/1765737/","msgid":"<CAPnjgZ0fEFOvnqt+_J6rh-eoas0EevVXaV_8u5N0WM5JkGdVjQ@mail.gmail.com>","list_archive_url":null,"date":"2017-09-09T04:55:00","subject":"Re: [U-Boot] [PATCH v2 2/8] fs/fat: introduce new director iterators","submitter":{"id":6170,"url":"http://patchwork.ozlabs.org/api/people/6170/","name":"Simon Glass","email":"sjg@chromium.org"},"content":"Hi Rob,\n\nOn 5 September 2017 at 03:54, Rob Clark <robdclark@gmail.com> wrote:\n> On Tue, Sep 5, 2017 at 4:56 AM, Simon Glass <sjg@chromium.org> wrote:\n>> Hi Rob,\n>>\n>> On 3 September 2017 at 00:37, Rob Clark <robdclark@gmail.com> wrote:\n>>> Untangle directory traversal into a simple iterator, to replace the\n>>> existing multi-purpose do_fat_read_at() + get_dentfromdir().\n>>>\n>>> Signed-off-by: Rob Clark <robdclark@gmail.com>\n>>> ---\n>>>  fs/fat/fat.c | 326 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n>>>  1 file changed, 326 insertions(+)\n>>>\n>>> diff --git a/fs/fat/fat.c b/fs/fat/fat.c\n>>> index e1c0a15dc7..c72d6ca931 100644\n>>> --- a/fs/fat/fat.c\n>>> +++ b/fs/fat/fat.c\n>>> @@ -1245,6 +1245,332 @@ exit:\n>>>         return ret;\n>>>  }\n>>>\n>>> +\n>>> +/*\n>>> + * Directory iterator, to simplify filesystem traversal\n>>> + */\n>>> +\n>>> +typedef struct {\n>>\n>> Please avoid using a typedef here. It seems like it could just be a struct.\n>\n> I'm typically not a fan of 'typedef struct' but that seems to be the\n> style that fs/fat code was already using, and \"when in Rome...\"\n\nSure, but let's move to Venice as it is less dusty.\n\n>\n>> Also pleaee add a comment about what the struct is for\n>>\n>>> +       fsdata    *fsdata;\n>>> +       unsigned   cursect;\n>>> +       dir_entry *dent;\n>>> +       int        remaining;     /* remaining dent's in current cluster */\n>>> +       int        last_cluster;\n>>> +       int        is_root;\n>>> +\n>>> +       /* current iterator position values: */\n>>> +       char       l_name[VFAT_MAXLEN_BYTES];\n>>> +       char       s_name[14];\n>>> +       char      *name;          /* l_name if there is one, else s_name */\n>>> +\n>>> +       u8         block[MAX_CLUSTSIZE] __aligned(ARCH_DMA_MINALIGN);\n>>\n>> Some members are missing comments.\n>>\n>> Also I'm not too sure how the alignment works here. I don't see you\n>> creating one of these objects in this patch, but could you add a\n>> comment to the struct about how to create one? Do you use memalign()?\n>\n> I was just creating them on the stack normally.. expecting the\n> compiler to dtrt because of the __aligned() on block[].\n\nWell I think that works. OK.\n\nRegards,\nSimon","headers":{"Return-Path":"<u-boot-bounces@lists.denx.de>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.denx.de\n\t(client-ip=81.169.180.215; helo=lists.denx.de;\n\tenvelope-from=u-boot-bounces@lists.denx.de;\n\treceiver=<UNKNOWN>)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=google.com header.i=@google.com\n\theader.b=\"AUxspx8Z\"; \n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"ki0uAI9K\"; dkim-atps=neutral"],"Received":["from lists.denx.de (dione.denx.de [81.169.180.215])\n\tby ozlabs.org (Postfix) with ESMTP id 3xq2Np2PLCz9sBZ\n\tfor <incoming@patchwork.ozlabs.org>;\n\tSat,  9 Sep 2017 15:11:54 +1000 (AEST)","by lists.denx.de (Postfix, from userid 105)\n\tid 0C49CC21F6A; Sat,  9 Sep 2017 05:03:12 +0000 (UTC)","from lists.denx.de (localhost [IPv6:::1])\n\tby lists.denx.de (Postfix) with ESMTP id 363FCC21C46;\n\tSat,  9 Sep 2017 04:57:31 +0000 (UTC)","by lists.denx.de (Postfix, from userid 105)\n\tid B62CAC21F44; Sat,  9 Sep 2017 04:56:33 +0000 (UTC)","from mail-qk0-f181.google.com (mail-qk0-f181.google.com\n\t[209.85.220.181])\n\tby lists.denx.de (Postfix) with ESMTPS id C7EE3C21F3A\n\tfor <u-boot@lists.denx.de>; Sat,  9 Sep 2017 04:55:22 +0000 (UTC)","by mail-qk0-f181.google.com with SMTP id r141so10458187qke.2\n\tfor <u-boot@lists.denx.de>; Fri, 08 Sep 2017 21:55:22 -0700 (PDT)","by 10.200.37.200 with HTTP; Fri, 8 Sep 2017 21:55:00 -0700 (PDT)"],"X-Spam-Checker-Version":"SpamAssassin 3.4.0 (2014-02-07) on lists.denx.de","X-Spam-Level":"","X-Spam-Status":"No, score=-0.0 required=5.0 tests=RCVD_IN_DNSWL_NONE,\n\tRCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL,\n\tT_DKIM_INVALID autolearn=unavailable\n\tautolearn_force=no version=3.4.0","DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com;\n\ts=20161025; \n\th=mime-version:sender:in-reply-to:references:from:date:message-id\n\t:subject:to:cc;\n\tbh=nFaowKLpJULNrGUkgkP6jRwXHgo0sLywyebvrEgmq9I=;\n\tb=AUxspx8ZrWT3JVwbTdIRBXfWPBQ8gIIL7Z65bNCmC5liIceNl1jfSaZic6hrhYg6jr\n\tHcSKpo/Vxggp/7BcmKuoJj+ygfoaDhsrgd13S1I+Xq30IVldfnZGct+/lZDk0EMD0G+N\n\t639sRyA5wxCT70bxdqaMrGcFGhVEWeZngTJSQmmuhWQf81nNVIvoHgfH8Rz5FvtsbPm7\n\tIW0YWmpSH1c1l07V/XoW2okZlAQKnB0U5X/vF+9b58f85x1BEP09P/+pglRE91DiI+ho\n\tTrlteZUGwXYyLLQj8WGLr/sYEA01mojbMt80XRAmIiZk6OZ3r3tsErzB+i7um3Hk2DE/\n\tiO+A==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:sender:in-reply-to:references:from:date:message-id\n\t:subject:to:cc;\n\tbh=nFaowKLpJULNrGUkgkP6jRwXHgo0sLywyebvrEgmq9I=;\n\tb=ki0uAI9Ko3AP4EOe8gR5xT99n6iwJXOeo82UCHaGBooXWHeekN5Zm01q1Pr/Nsz/vk\n\t4uEg1Ki3KXPDGtj0w4T4Rn90m1S3rt7LGERgnp6HcqRfu9dfoFLyB1bNY6/n+/xvuGOu\n\tKFdlTWsjJ7Sok3UYI32rb/+Wc7hbrdcHiVcA8="],"X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:sender:in-reply-to:references:from\n\t:date:message-id:subject:to:cc;\n\tbh=nFaowKLpJULNrGUkgkP6jRwXHgo0sLywyebvrEgmq9I=;\n\tb=jeGZn2DUFzb2rgEARcXEmTrUAJU8E5OU5m90oiwWNbNHSl/f2z3CMHnk7I6gypl4Sj\n\tG7nIQWrH/r3oOwALzrJAjQeY7ap8K1LesKt3L5ZHq00euMNm6tYoop9YfzQ0WOxAy0eV\n\tYNDlOkJqa3V1GOJwM/biQ/kl57cv9xz4JweCK2sjEaPsBkipvyQZ5TkY+vadJ0765snx\n\tXYXSoEvi+rJdo4hQMAiuCW5DMQKRKkOjXug2hj6Vpg2DKYh+uPxv8vzcl+oiUUW5eQUB\n\ti8LpPTXrXcpbWWW9V7zX+epz8zpF5iTI3D2Oapca4M39TzZY+KZVdS231gFeHThsL98l\n\tdjuw==","X-Gm-Message-State":"AHPjjUjjC13PdLUukaCqyPuHgyp2gtAKwquibfW/ea3wZdZCG4joVHdB\n\toWIZqycaYV8aBme5UgsmYfjYd59C91q1Ov1QbVGntYb8","X-Google-Smtp-Source":"AOwi7QBej6WPFhNZ+RXDo8T/vVw1SIeWKbysQ5zOBIER88fsO6lmWyFGBKFr+MbAbHfw36/KZV+izyI+HJllxgXfcHE=","X-Received":"by 10.55.146.198 with SMTP id u189mr6534680qkd.317.1504932921444;\n\tFri, 08 Sep 2017 21:55:21 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"<CAF6AEGuoT=XhgBZKjzxqmjgRKXpto+nZXrz=XfixqVYzVYF4Ww@mail.gmail.com>","References":"<20170902163806.27265-1-robdclark@gmail.com>\n\t<20170902163806.27265-3-robdclark@gmail.com>\n\t<CAPnjgZ1vNYbVL7qvOUjJzNBdO0F40qvdHCOhAiYruRwM2jsfOA@mail.gmail.com>\n\t<CAF6AEGuoT=XhgBZKjzxqmjgRKXpto+nZXrz=XfixqVYzVYF4Ww@mail.gmail.com>","From":"Simon Glass <sjg@chromium.org>","Date":"Fri, 8 Sep 2017 22:55:00 -0600","X-Google-Sender-Auth":"0jW0t80tTJ5eHzjMhC6gtYmw5Ko","Message-ID":"<CAPnjgZ0fEFOvnqt+_J6rh-eoas0EevVXaV_8u5N0WM5JkGdVjQ@mail.gmail.com>","To":"Rob Clark <robdclark@gmail.com>","Cc":"U-Boot Mailing List <u-boot@lists.denx.de>","Subject":"Re: [U-Boot] [PATCH v2 2/8] fs/fat: introduce new director iterators","X-BeenThere":"u-boot@lists.denx.de","X-Mailman-Version":"2.1.18","Precedence":"list","List-Id":"U-Boot discussion <u-boot.lists.denx.de>","List-Unsubscribe":"<https://lists.denx.de/options/u-boot>,\n\t<mailto:u-boot-request@lists.denx.de?subject=unsubscribe>","List-Archive":"<http://lists.denx.de/pipermail/u-boot/>","List-Post":"<mailto:u-boot@lists.denx.de>","List-Help":"<mailto:u-boot-request@lists.denx.de?subject=help>","List-Subscribe":"<https://lists.denx.de/listinfo/u-boot>,\n\t<mailto:u-boot-request@lists.denx.de?subject=subscribe>","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"u-boot-bounces@lists.denx.de","Sender":"\"U-Boot\" <u-boot-bounces@lists.denx.de>"}},{"id":1765782,"web_url":"http://patchwork.ozlabs.org/comment/1765782/","msgid":"<CAF6AEGtY7zVxwO9OnVNUF0D4Qic51=N6=Gos792iCyJZvqmczA@mail.gmail.com>","list_archive_url":null,"date":"2017-09-09T10:34:55","subject":"Re: [U-Boot] [PATCH v2 2/8] fs/fat: introduce new director iterators","submitter":{"id":18760,"url":"http://patchwork.ozlabs.org/api/people/18760/","name":"Rob Clark","email":"robdclark@gmail.com"},"content":"On Sat, Sep 9, 2017 at 12:55 AM, Simon Glass <sjg@chromium.org> wrote:\n> Hi Rob,\n>\n> On 5 September 2017 at 03:54, Rob Clark <robdclark@gmail.com> wrote:\n>> On Tue, Sep 5, 2017 at 4:56 AM, Simon Glass <sjg@chromium.org> wrote:\n>>> Hi Rob,\n>>>\n>>> On 3 September 2017 at 00:37, Rob Clark <robdclark@gmail.com> wrote:\n>>>> Untangle directory traversal into a simple iterator, to replace the\n>>>> existing multi-purpose do_fat_read_at() + get_dentfromdir().\n>>>>\n>>>> Signed-off-by: Rob Clark <robdclark@gmail.com>\n>>>> ---\n>>>>  fs/fat/fat.c | 326 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n>>>>  1 file changed, 326 insertions(+)\n>>>>\n>>>> diff --git a/fs/fat/fat.c b/fs/fat/fat.c\n>>>> index e1c0a15dc7..c72d6ca931 100644\n>>>> --- a/fs/fat/fat.c\n>>>> +++ b/fs/fat/fat.c\n>>>> @@ -1245,6 +1245,332 @@ exit:\n>>>>         return ret;\n>>>>  }\n>>>>\n>>>> +\n>>>> +/*\n>>>> + * Directory iterator, to simplify filesystem traversal\n>>>> + */\n>>>> +\n>>>> +typedef struct {\n>>>\n>>> Please avoid using a typedef here. It seems like it could just be a struct.\n>>\n>> I'm typically not a fan of 'typedef struct' but that seems to be the\n>> style that fs/fat code was already using, and \"when in Rome...\"\n>\n> Sure, but let's move to Venice as it is less dusty.\n>\n\nI'm not a huge fan of mixing styles.. but what I'd propose instead is this:\n\nIt turns out fat_write.c needs a lot of work for SCT too (ability to\nwrite files in subdirectories, and write files w/ !=0 offset), so that\nwe can actually see the test results.. and before converting write\npaths to use the new directory iterator, I was considering\nre-organizing the code a bit, ie. move directory traversal code to\nfs/fat/dir.c or util.c, move most of include/fat.h into a private\nheader, dropping the #include \"fat.c\" hack, etc.  That would be a good\ntime for flag-day cleanups like untypedef'ifying and perhaps cleaning\nup all the #define macros that implicitly depend on having a 'mydata'\nvar.\n\nSo I'd suggest to leave it as-is for now, and change things to 'struct\nfoo' as part of the re-org.\n\nThat leaves FS_DIR in the later readdir patch.. I could change that to\n'struct fs_dir_stream'.  That makes it not match the posix API it is\nmodelled after as closely, but maybe that is ok.\n\nBR,\n-R","headers":{"Return-Path":"<u-boot-bounces@lists.denx.de>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.denx.de\n\t(client-ip=81.169.180.215; helo=lists.denx.de;\n\tenvelope-from=u-boot-bounces@lists.denx.de;\n\treceiver=<UNKNOWN>)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"uzfyJC/B\"; dkim-atps=neutral"],"Received":["from lists.denx.de (dione.denx.de [81.169.180.215])\n\tby ozlabs.org (Postfix) with ESMTP id 3xq9Yr3Bwcz9sBW\n\tfor <incoming@patchwork.ozlabs.org>;\n\tSat,  9 Sep 2017 20:35:11 +1000 (AEST)","by lists.denx.de (Postfix, from userid 105)\n\tid 0A370C21E7C; Sat,  9 Sep 2017 10:35:01 +0000 (UTC)","from lists.denx.de (localhost [IPv6:::1])\n\tby lists.denx.de (Postfix) with ESMTP id 76947C21D7D;\n\tSat,  9 Sep 2017 10:34:59 +0000 (UTC)","by lists.denx.de (Postfix, from userid 105)\n\tid 57C7FC21D7D; Sat,  9 Sep 2017 10:34:57 +0000 (UTC)","from mail-lf0-f47.google.com (mail-lf0-f47.google.com\n\t[209.85.215.47])\n\tby lists.denx.de (Postfix) with ESMTPS id E2DB1C21C93\n\tfor <u-boot@lists.denx.de>; Sat,  9 Sep 2017 10:34:56 +0000 (UTC)","by mail-lf0-f47.google.com with SMTP id m199so9939077lfe.3\n\tfor <u-boot@lists.denx.de>; Sat, 09 Sep 2017 03:34:56 -0700 (PDT)","by 10.46.29.20 with HTTP; Sat, 9 Sep 2017 03:34:55 -0700 (PDT)"],"X-Spam-Checker-Version":"SpamAssassin 3.4.0 (2014-02-07) on lists.denx.de","X-Spam-Level":"","X-Spam-Status":"No, score=-0.0 required=5.0 tests=FREEMAIL_FROM,\n\tRCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL,\n\tT_DKIM_INVALID\n\tautolearn=unavailable autolearn_force=no version=3.4.0","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;\n\th=mime-version:in-reply-to:references:from:date:message-id:subject:to\n\t:cc; bh=t+qXv2Zh0lghOW1DvxV1UtZ2gAoSeh5OrtT4lZqdBUU=;\n\tb=uzfyJC/B6w+8iHtw0arkTtIGgUzn7k/MGc0TDgd4YIDWFe7+BGdu6/41If3P+wF23j\n\tpr/C1xLFVLYN3IcnIvQCAw+XLoyL0Cecea4nWU1hFH3AYiNigMF7jx5mKWZHjAKzYOac\n\tIrPRaqT/38ek3woyzahYakewBy1VihvChU3y9UfFNrHDrXXNPGnV094cUu8AbOwdPEcz\n\tQ/iZG+2ooeMsa/BTcx1lRMjNfUX7Ut7k3cCb//vk4qxGmAdiFZn5xayun373O+mvYqfh\n\tJvErxZXBd4pem0gSviXo6qa89QEzLrjdN/Pco5u2eWhrsuzmK/09cXy8LLZRhmtKkOCH\n\tqW1w==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:in-reply-to:references:from:date\n\t:message-id:subject:to:cc;\n\tbh=t+qXv2Zh0lghOW1DvxV1UtZ2gAoSeh5OrtT4lZqdBUU=;\n\tb=RAL2y99LOqZYG8SOA1PhE7hUheo/y+5vU9INoFHcT7386ZREP4aNT388WzdkuqThtF\n\tfbsM+52dymJkNarF2YZ1eoQksTWlQfEj/oSZHfpu0RUyx4zCyIXF5U/gvE6pUFD8r3Vr\n\tAwFxbWgT802JI49OA11gUK8b/WU5ByQ/Q/M10jB457kXAVcJJtHzyACzMCcmqnpnQ6ij\n\thN4b9BjyxtoXLykXIH3saAolDGBZy44rliNacBmKzW02vIBmjHu90L2LOARgMvXMjNm/\n\tU7dMa7vLfSMvw/XBPmPVhv4DvwANHf5P0Wb+zhUvvKqMCVRVQ56GL+TjBeBpcgs3MYTd\n\txfQw==","X-Gm-Message-State":"AHPjjUhRm5cB4ZqPT4a19HDAJ9P3a1SsX9xK8dqVvHHa0k27hzjhPQWU\n\t0UZ3PXsPNlKM00TlhNThVWp9kzfNDw==","X-Google-Smtp-Source":"AOwi7QDP3Oi4as2u1/TlVXIm3J43swFsPqRlqVPza9QKMklYqgy4ae1WvPB71FSObFKs8tsRKQIf0DPJYetaWYX6gE4=","X-Received":"by 10.25.23.105 with SMTP id n102mr2018394lfi.252.1504953296373; \n\tSat, 09 Sep 2017 03:34:56 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"<CAPnjgZ0fEFOvnqt+_J6rh-eoas0EevVXaV_8u5N0WM5JkGdVjQ@mail.gmail.com>","References":"<20170902163806.27265-1-robdclark@gmail.com>\n\t<20170902163806.27265-3-robdclark@gmail.com>\n\t<CAPnjgZ1vNYbVL7qvOUjJzNBdO0F40qvdHCOhAiYruRwM2jsfOA@mail.gmail.com>\n\t<CAF6AEGuoT=XhgBZKjzxqmjgRKXpto+nZXrz=XfixqVYzVYF4Ww@mail.gmail.com>\n\t<CAPnjgZ0fEFOvnqt+_J6rh-eoas0EevVXaV_8u5N0WM5JkGdVjQ@mail.gmail.com>","From":"Rob Clark <robdclark@gmail.com>","Date":"Sat, 9 Sep 2017 06:34:55 -0400","Message-ID":"<CAF6AEGtY7zVxwO9OnVNUF0D4Qic51=N6=Gos792iCyJZvqmczA@mail.gmail.com>","To":"Simon Glass <sjg@chromium.org>","Cc":"U-Boot Mailing List <u-boot@lists.denx.de>","Subject":"Re: [U-Boot] [PATCH v2 2/8] fs/fat: introduce new director iterators","X-BeenThere":"u-boot@lists.denx.de","X-Mailman-Version":"2.1.18","Precedence":"list","List-Id":"U-Boot discussion <u-boot.lists.denx.de>","List-Unsubscribe":"<https://lists.denx.de/options/u-boot>,\n\t<mailto:u-boot-request@lists.denx.de?subject=unsubscribe>","List-Archive":"<http://lists.denx.de/pipermail/u-boot/>","List-Post":"<mailto:u-boot@lists.denx.de>","List-Help":"<mailto:u-boot-request@lists.denx.de?subject=help>","List-Subscribe":"<https://lists.denx.de/listinfo/u-boot>,\n\t<mailto:u-boot-request@lists.denx.de?subject=subscribe>","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"u-boot-bounces@lists.denx.de","Sender":"\"U-Boot\" <u-boot-bounces@lists.denx.de>"}}]