[{"id":1762285,"web_url":"http://patchwork.ozlabs.org/comment/1762285/","msgid":"<6856fa6d-a7ab-0ed4-1f41-98b40283dfb0@denx.de>","list_archive_url":null,"date":"2017-09-03T15:22:05","subject":"Re: [U-Boot] [PATCH v2 8/8] fs/fat: fix case for FAT shortnames","submitter":{"id":70701,"url":"http://patchwork.ozlabs.org/api/people/70701/","name":"Lukasz Majewski","email":"lukma@denx.de"},"content":"On 09/02/2017 06:38 PM, Rob Clark wrote:\n> Noticed when comparing our output to linux. There are some lcase bits\n> which control whether filename and/or extension should be downcase'd.\n\nReviewed-by: Łukasz Majewski \n\n> \n> Signed-off-by: Rob Clark \n> ---\n> fs/fat/fat.c | 17 ++++++++++++-----\n> fs/fat/fat_write.c | 4 ++--\n> include/fat.h | 3 +++\n> 3 files changed, 17 insertions(+), 7 deletions(-)\n> \n> diff --git a/fs/fat/fat.c b/fs/fat/fat.c\n> index fc3106aacb..ccbf7ba1c8 100644\n> --- a/fs/fat/fat.c\n> +++ b/fs/fat/fat.c\n> @@ -29,11 +29,13 @@ static const int vfat_enabled = 0;\n> #endif\n> \n> /*\n> - * Convert a string to lowercase.\n> + * Convert a string to lowercase. Converts at most 'len' characters,\n> + * 'len' may be larger than the length of 'str' if 'str' is NULL\n> + * terminated.\n> */\n> -static void downcase(char *str)\n> +static void downcase(char *str, size_t len)\n> {\n> -\twhile (*str != '\\0') {\n> +\twhile (*str != '\\0' && len--) {\n> \t\t*str = tolower(*str);\n> \t\tstr++;\n> \t}\n> @@ -131,10 +133,16 @@ static void get_name(dir_entry *dirent, char *s_name)\n> \tptr = s_name;\n> \twhile (*ptr && *ptr != ' ')\n> \t\tptr++;\n> +\tif (dirent->lcase & CASE_LOWER_BASE)\n> +\t\tdowncase(s_name, (unsigned)(ptr - s_name));\n> \tif (dirent->ext[0] && dirent->ext[0] != ' ') {\n> +\t\tchar *ext;\n> +\n> \t\t*ptr = '.';\n> -\t\tptr++;\n> +\t\text = ++ptr;\n> \t\tmemcpy(ptr, dirent->ext, 3);\n> +\t\tif (dirent->lcase & CASE_LOWER_EXT)\n> +\t\t\tdowncase(ext, 3);\n> \t\tptr[3] = '\\0';\n> \t\twhile (*ptr && *ptr != ' ')\n> \t\t\tptr++;\n> @@ -144,7 +152,6 @@ static void get_name(dir_entry *dirent, char *s_name)\n> \t\t*s_name = '\\0';\n> \telse if (*s_name == aRING)\n> \t\t*s_name = DELETED_FLAG;\n> -\tdowncase(s_name);\n> }\n> \n> static int flush_dirty_fat_buffer(fsdata *mydata);\n> diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c\n> index 4ca024c208..655ad4ec84 100644\n> --- a/fs/fat/fat_write.c\n> +++ b/fs/fat/fat_write.c\n> @@ -345,7 +345,7 @@ get_long_file_name(fsdata *mydata, int curclust, __u8 *cluster,\n> \t\t*l_name = '\\0';\n> \telse if (*l_name == aRING)\n> \t\t*l_name = DELETED_FLAG;\n> -\tdowncase(l_name);\n> +\tdowncase(l_name, ~0);\n> \n> \t/* Return the real directory entry */\n> \t*retdent = realdent;\n> @@ -981,7 +981,7 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size,\n> \n> \tmemcpy(l_filename, filename, name_len);\n> \tl_filename[name_len] = 0; /* terminate the string */\n> -\tdowncase(l_filename);\n> +\tdowncase(l_filename, ~0);\n> \n> \tstartsect = mydata->rootdir_sect;\n> \tretdent = find_directory_entry(mydata, startsect,\n> diff --git a/include/fat.h b/include/fat.h\n> index b2d4b952fd..5e4924316a 100644\n> --- a/include/fat.h\n> +++ b/include/fat.h\n> @@ -128,6 +128,9 @@ typedef struct volume_info\n> \t/* Boot sign comes last, 2 bytes */\n> } volume_info;\n> \n> +#define CASE_LOWER_BASE\t8\t/* base is lower case */\n> +#define CASE_LOWER_EXT\t16\t/* extension is lower case */\n> +\n> typedef struct dir_entry {\n> \tchar\tname[8],ext[3];\t/* Name and extension */\n> \t__u8\tattr;\t\t/* Attribute bits */\n>","headers":{"Return-Path":"","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=)","Received":["from lists.denx.de (dione.denx.de [81.169.180.215])\n\tby ozlabs.org (Postfix) with ESMTP id 3xlcCs5kMqz9t33\n\tfor ;\n\tMon, 4 Sep 2017 01:22:17 +1000 (AEST)","by lists.denx.de (Postfix, from userid 105)\n\tid 8D496C21F1D; Sun, 3 Sep 2017 15:22:15 +0000 (UTC)","from lists.denx.de (localhost [IPv6:::1])\n\tby lists.denx.de (Postfix) with ESMTP id 9BD00C21DBD;\n\tSun, 3 Sep 2017 15:22:12 +0000 (UTC)","by lists.denx.de (Postfix, from userid 105)\n\tid 16CA9C21DBD; Sun, 3 Sep 2017 15:22:11 +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 B596EC21D79\n\tfor ; Sun, 3 Sep 2017 15:22:10 +0000 (UTC)","from frontend01.mail.m-online.net (unknown [192.168.8.182])\n\tby mail-out.m-online.net (Postfix) with ESMTP id 3xlcCg6StNz1qw7N;\n\tSun, 3 Sep 2017 17:22:07 +0200 (CEST)","from localhost (dynscan1.mnet-online.de [192.168.6.70])\n\tby mail.m-online.net (Postfix) with ESMTP id 3xlcCg1qM1z3hjkw;\n\tSun, 3 Sep 2017 17:22: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 D5skt35F_3cg; Sun, 3 Sep 2017 17:22: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:22:05 +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":"5PeN5zUsRyEBmPR6AOtD7KDCB5m2jy9dsKm/0b56MVA=","To":"Rob Clark ,\n\tU-Boot Mailing List ","References":"<20170902163806.27265-1-robdclark@gmail.com>\n\t<20170902163806.27265-9-robdclark@gmail.com>","From":"=?utf-8?q?=C5=81ukasz_Majewski?= ","Organization":"DENX","Message-ID":"<6856fa6d-a7ab-0ed4-1f41-98b40283dfb0@denx.de>","Date":"Sun, 3 Sep 2017 17:22: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-9-robdclark@gmail.com>","Content-Language":"en-US","Cc":"Tien Fong Chee , Stephen Warren , \n\tGenevieve Chan ,\n\tPhilipp Skadorov ","Subject":"Re: [U-Boot] [PATCH v2 8/8] fs/fat: fix case for FAT shortnames","X-BeenThere":"u-boot@lists.denx.de","X-Mailman-Version":"2.1.18","Precedence":"list","List-Id":"U-Boot discussion ","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Transfer-Encoding":"base64","Content-Type":"text/plain; charset=\"utf-8\"; Format=\"flowed\"","Errors-To":"u-boot-bounces@lists.denx.de","Sender":"\"U-Boot\" "}},{"id":1763102,"web_url":"http://patchwork.ozlabs.org/comment/1763102/","msgid":"","list_archive_url":null,"date":"2017-09-05T08:56:41","subject":"Re: [U-Boot] [PATCH v2 8/8] fs/fat: fix case for FAT shortnames","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 23:22, Łukasz Majewski wrote:\n> On 09/02/2017 06:38 PM, Rob Clark wrote:\n>>\n>> Noticed when comparing our output to linux. There are some lcase bits\n>> which control whether filename and/or extension should be downcase'd.\n>\n>\n> Reviewed-by: Łukasz Majewski \n>\n>\n>>\n>> Signed-off-by: Rob Clark \n>> ---\n>> fs/fat/fat.c | 17 ++++++++++++-----\n>> fs/fat/fat_write.c | 4 ++--\n>> include/fat.h | 3 +++\n>> 3 files changed, 17 insertions(+), 7 deletions(-)\n>>\n>> diff --git a/fs/fat/fat.c b/fs/fat/fat.c\n>> index fc3106aacb..ccbf7ba1c8 100644\n>> --- a/fs/fat/fat.c\n>> +++ b/fs/fat/fat.c\n>> @@ -29,11 +29,13 @@ static const int vfat_enabled = 0;\n>> #endif\n>> /*\n>> - * Convert a string to lowercase.\n>> + * Convert a string to lowercase. Converts at most 'len' characters,\n>> + * 'len' may be larger than the length of 'str' if 'str' is NULL\n>> + * terminated.\n>> */\n>> -static void downcase(char *str)\n>> +static void downcase(char *str, size_t len)\n>> {\n>> - while (*str != '\\0') {\n>> + while (*str != '\\0' && len--) {\n>> *str = tolower(*str);\n>> str++;\n>> }\n>> @@ -131,10 +133,16 @@ static void get_name(dir_entry *dirent, char\n>> *s_name)\n>> ptr = s_name;\n>> while (*ptr && *ptr != ' ')\n>> ptr++;\n>> + if (dirent->lcase & CASE_LOWER_BASE)\n>> + downcase(s_name, (unsigned)(ptr - s_name));\n>> if (dirent->ext[0] && dirent->ext[0] != ' ') {\n>> + char *ext;\n>> +\n>> *ptr = '.';\n>> - ptr++;\n>> + ext = ++ptr;\n\nI think this would be clearer as:\n\n *ptr++ = '.';\n ext = ptr;\n\nBut given that you only use ext once, can you just drop it?\n\n>> memcpy(ptr, dirent->ext, 3);\n>> + if (dirent->lcase & CASE_LOWER_EXT)\n>> + downcase(ext, 3);\n>> ptr[3] = '\\0';\n>> while (*ptr && *ptr != ' ')\n>> ptr++;\n>> @@ -144,7 +152,6 @@ static void get_name(dir_entry *dirent, char *s_name)\n>> *s_name = '\\0';\n>> else if (*s_name == aRING)\n>> *s_name = DELETED_FLAG;\n>> - downcase(s_name);\n>> }\n>> static int flush_dirty_fat_buffer(fsdata *mydata);\n>> diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c\n>> index 4ca024c208..655ad4ec84 100644\n>> --- a/fs/fat/fat_write.c\n>> +++ b/fs/fat/fat_write.c\n>> @@ -345,7 +345,7 @@ get_long_file_name(fsdata *mydata, int curclust, __u8\n>> *cluster,\n>> *l_name = '\\0';\n>> else if (*l_name == aRING)\n>> *l_name = DELETED_FLAG;\n>> - downcase(l_name);\n>> + downcase(l_name, ~0);\n\nCould this be INT_MAX to indicate all? I think ~0 is a bit obscure\n\n>> /* Return the real directory entry */\n>> *retdent = realdent;\n>> @@ -981,7 +981,7 @@ static int do_fat_write(const char *filename, void\n>> *buffer, loff_t size,\n>> memcpy(l_filename, filename, name_len);\n>> l_filename[name_len] = 0; /* terminate the string */\n>> - downcase(l_filename);\n>> + downcase(l_filename, ~0);\n\nhere also\n\n>> startsect = mydata->rootdir_sect;\n>> retdent = find_directory_entry(mydata, startsect,\n>> diff --git a/include/fat.h b/include/fat.h\n>> index b2d4b952fd..5e4924316a 100644\n>> --- a/include/fat.h\n>> +++ b/include/fat.h\n>> @@ -128,6 +128,9 @@ typedef struct volume_info\n>> /* Boot sign comes last, 2 bytes */\n>> } volume_info;\n>> +#define CASE_LOWER_BASE 8 /* base is lower case */\n>> +#define CASE_LOWER_EXT 16 /* extension is lower case */\n\nWhere are these two used? If they are flags can you mention somewhere\nwhat uses them?\n\n>> +\n>> typedef struct dir_entry {\n>> char name[8],ext[3]; /* Name and extension */\n>> __u8 attr; /* Attribute bits */\n>>\n>\n\nRegards,\nSimon","headers":{"Return-Path":"","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=)","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=\"M3HK7Z0X\"; \n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"Mm5ogTdH\"; dkim-atps=neutral"],"Received":["from lists.denx.de (dione.denx.de [81.169.180.215])\n\tby ozlabs.org (Postfix) with ESMTP id 3xmgqr1TvFz9sPk\n\tfor ;\n\tTue, 5 Sep 2017 19:08:40 +1000 (AEST)","by lists.denx.de (Postfix, from userid 105)\n\tid EEFF4C21F90; Tue, 5 Sep 2017 09:06:04 +0000 (UTC)","from lists.denx.de (localhost [IPv6:::1])\n\tby lists.denx.de (Postfix) with ESMTP id A7682C21F59;\n\tTue, 5 Sep 2017 09:03:33 +0000 (UTC)","by lists.denx.de (Postfix, from userid 105)\n\tid 2BF5BC21F39; Tue, 5 Sep 2017 08:57:08 +0000 (UTC)","from mail-qt0-f172.google.com (mail-qt0-f172.google.com\n\t[209.85.216.172])\n\tby lists.denx.de (Postfix) with ESMTPS id 6FBF3C21DED\n\tfor ; Tue, 5 Sep 2017 08:57:03 +0000 (UTC)","by mail-qt0-f172.google.com with SMTP id h15so9758136qta.4\n\tfor ; Tue, 05 Sep 2017 01:57:03 -0700 (PDT)","by 10.200.28.108 with HTTP; Tue, 5 Sep 2017 01:56:41 -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:content-transfer-encoding;\n\tbh=zrb5krpbzhTWmQlc5G47ejKIdapX54ETKKAkS0LioZg=;\n\tb=M3HK7Z0XO+gXwaEF2agsdM/hu0adbq3KDv3+pXcgYMIfYDyqFlqInQLzigXb5KWzQ4\n\tsX6k9v7TBtpcv2sYiI7rv69gFhnDvAEuYEawXwuUYl/5rVW5/sNwBI87Xf0ITtL0D4Eo\n\t+FsQizcaiZFpH+tip568dxcwrf0bdFaIqOXjivUCOo2161kNfE1ewcKr33x1aGOwjn3p\n\t0SAl5CjNERyjxbrzGRXNNW+PNFsYGaFd+l8qs1UtjN2nIdqVpi0IjPZ1iyqL0VhY3wUl\n\tJf3bBNqmDSNCH8EwuCcmPEeAI7fxsRo3gGySNZgCb/Y8uy9BZSwwNQ/4obgN22y4qb5Y\n\tD4lA==","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:content-transfer-encoding;\n\tbh=zrb5krpbzhTWmQlc5G47ejKIdapX54ETKKAkS0LioZg=;\n\tb=Mm5ogTdHin/jfZSrxAKMh0ATOLu1WYPVB9MHKxu0EF9k9IJtZypfioMo5SOJ8oWDlS\n\tBNaunU5+pgyfZGn6ZW60AIE3lR0g7LLDfk8s6VuMqYAqy/Z0jFMWFAxDohnJ7Oh1eHNT\n\t23e72/c8VpGOiwIISH+RZztDvpSbhiR1J39DU="],"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:content-transfer-encoding;\n\tbh=zrb5krpbzhTWmQlc5G47ejKIdapX54ETKKAkS0LioZg=;\n\tb=SkQgL4BSRWDI98NteBKfxxMHI/1dBNtKVv2tzX2lWCWSK7l/niktlucc8otO898hLN\n\tvApP7OTNZ51fgiwTP2AWU8FJrtdVTVQw11gSKTtmfEH51osUJDq0YAdmIBVdGMiqlR19\n\t337xyN7c+Y0bxGlfrYd/cYufyVJB6aWThQLwHjuCo7zx5Ex4CUa48sUbnTIBebRhHyce\n\twJhTQj1D4/wKTkLZ07r68K76Nc9v/37UIy9elhEJ80l0nvdM1IHKgjrjjaucPaUlSQMk\n\td3tj9FspTkNKRgpl1xmARWtfH6gyMjsnusbVTy7tXi0XL6Z6KdoBUgFYrbA4Gd6B+pFH\n\tWdxA==","X-Gm-Message-State":"AHPjjUgRidVH+A8lizRiYvWS0mWe5rzxODUb1Myo4wuFF5Ifityt2PEI\n\tgrZqdtNIwO4eKtbrGgWvf3HIDI71C+Nx","X-Google-Smtp-Source":"ADKCNb4wIZlX+/Hu1xUzgv5bakVBkpD0hdf8X708V66tR9x3XiUcusx/uM8dR+OrO53hb1qHorfon6E/768JcP3Tfh8=","X-Received":"by 10.200.46.138 with SMTP id h10mr8905qta.137.1504601822141;\n\tTue, 05 Sep 2017 01:57:02 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"<6856fa6d-a7ab-0ed4-1f41-98b40283dfb0@denx.de>","References":"<20170902163806.27265-1-robdclark@gmail.com>\n\t<20170902163806.27265-9-robdclark@gmail.com>\n\t<6856fa6d-a7ab-0ed4-1f41-98b40283dfb0@denx.de>","From":"Simon Glass ","Date":"Tue, 5 Sep 2017 16:56:41 +0800","X-Google-Sender-Auth":"CyNxB5HTHQITTYu6Xa7Wa2bYMSk","Message-ID":"","To":"=?utf-8?q?=C5=81ukasz_Majewski?= ","Cc":"Stephen Warren ,\n\tPhilipp Skadorov ,\n\tU-Boot Mailing List ,\n\tGenevieve Chan , Tien Fong Chee ","Subject":"Re: [U-Boot] [PATCH v2 8/8] fs/fat: fix case for FAT shortnames","X-BeenThere":"u-boot@lists.denx.de","X-Mailman-Version":"2.1.18","Precedence":"list","List-Id":"U-Boot discussion ","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"u-boot-bounces@lists.denx.de","Sender":"\"U-Boot\" "}}]